Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows-x86 flaky test failure: internal check failed in flush_impl: false #859

Closed
strager opened this issue Oct 7, 2022 · 5 comments
Closed
Assignees
Labels
false positive Severe bug: quick-lint-js rejects valid code

Comments

@strager
Copy link
Collaborator

strager commented Oct 7, 2022

https://github.com/quick-lint/quick-lint-js/actions/runs/3202094600/jobs/5230725143

test 1
    Start 1: quick-lint-js-test

1: Test command: D:\a\quick-lint-js\quick-lint-js\build\test\quick-lint-js-test.exe
1: Working Directory: D:/a/quick-lint-js/quick-lint-js/build/test
1: Test timeout computed to be: 1500
1: Running main() from gmock_main.cc
1: [==========] Running 1879 tests from 145 test suites.
1: [----------] Global test environment set-up.
1: [----------] 3 tests from test_write_integer
1: [ RUN      ] test_write_integer.common_integers
1: [       OK ] test_write_integer.common_integers (0 ms)
[snip]
1: [ RUN      ] typescript/test_parse_conditional_expression.conditional_expression_with_missing_colon_and_false_component/0
1: [       OK ] typescript/test_parse_conditional_expression.conditional_expression_with_missing_colon_and_false_component/0 (0 ms)
1: [----------] 5 tests from typescript/test_parse_conditional_expression (0 ms total)
1: 
1: [----------] Global test environment tear-down
1: [==========] 1879 tests from 145 test suites ran. (9726 ms total)
1: [  PASSED  ] 1879 tests.
1: 
1:   YOU HAVE 1 DISABLED TEST
1: 
1: D:/a/quick-lint-js/quick-lint-js/src/quick-lint-js/io/output-stream.cpp:101: internal check failed in flush_impl: false
1: quick-lint-js crashed. Please report this bug here:
1: https://quick-lint-js.com/crash-report/
1/2 Test #1: quick-lint-js-test ...............***Failed    9.77 sec
@strager strager added the false positive Severe bug: quick-lint-js rejects valid code label Oct 7, 2022
@strager
Copy link
Collaborator Author

strager commented Oct 7, 2022

@strager strager self-assigned this Oct 7, 2022
@strager
Copy link
Collaborator Author

strager commented Oct 9, 2022

strager added a commit that referenced this issue Oct 9, 2022
Tests are flaky on Windows [1]. Here's what I see on CI when a failure happens:

    [snip]
    1: [----------] 5 tests from typescript/test_parse_conditional_expression (0 ms total)
    1:
    1: [----------] Global test environment tear-down
    1: [==========] 1879 tests from 145 test suites ran. (9726 ms total)
    1: [  PASSED  ] 1879 tests.
    1:
    1:   YOU HAVE 1 DISABLED TEST
    1:
    1: D:/a/quick-lint-js/quick-lint-js/src/quick-lint-js/io/output-stream.cpp:101: internal check failed in flush_impl: false
    1: quick-lint-js crashed. Please report this bug here:
    1: https://quick-lint-js.com/crash-report/
    1/2 Test #1: quick-lint-js-test ...............***Failed    9.77 sec

Add some debug logs to help isolate the bug.

[1] #859
strager added a commit that referenced this issue Oct 9, 2022
Tests are flaky on Windows [1]. Here's what I see on CI when a failure happens:

    [snip]
    1: [----------] 5 tests from typescript/test_parse_conditional_expression (0 ms total)
    1:
    1: [----------] Global test environment tear-down
    1: [==========] 1879 tests from 145 test suites ran. (9726 ms total)
    1: [  PASSED  ] 1879 tests.
    1:
    1:   YOU HAVE 1 DISABLED TEST
    1:
    1: D:/a/quick-lint-js/quick-lint-js/src/quick-lint-js/io/output-stream.cpp:101: internal check failed in flush_impl: false
    1: quick-lint-js crashed. Please report this bug here:
    1: https://quick-lint-js.com/crash-report/
    1/2 Test #1: quick-lint-js-test ...............***Failed    9.77 sec

Add some debug logs to help isolate the bug.

[1] #859
strager added a commit that referenced this issue Oct 12, 2022
Tests are flaky on Windows [1]. Here's what I see on CI when a failure happens:

    [snip]
    1: [----------] 5 tests from typescript/test_parse_conditional_expression (0 ms total)
    1:
    1: [----------] Global test environment tear-down
    1: [==========] 1879 tests from 145 test suites ran. (9726 ms total)
    1: [  PASSED  ] 1879 tests.
    1:
    1:   YOU HAVE 1 DISABLED TEST
    1:
    1: D:/a/quick-lint-js/quick-lint-js/src/quick-lint-js/io/output-stream.cpp:101: internal check failed in flush_impl: false
    1: quick-lint-js crashed. Please report this bug here:
    1: https://quick-lint-js.com/crash-report/
    1/2 Test #1: quick-lint-js-test ...............***Failed    9.77 sec

Add some debug logs to help isolate the bug.

[1] #859
strager added a commit that referenced this issue Oct 12, 2022
Tests are flaky on Windows [1]. Here's what I see on CI when a failure happens:

    [snip]
    1: [----------] 5 tests from typescript/test_parse_conditional_expression (0 ms total)
    1:
    1: [----------] Global test environment tear-down
    1: [==========] 1879 tests from 145 test suites ran. (9726 ms total)
    1: [  PASSED  ] 1879 tests.
    1:
    1:   YOU HAVE 1 DISABLED TEST
    1:
    1: D:/a/quick-lint-js/quick-lint-js/src/quick-lint-js/io/output-stream.cpp:101: internal check failed in flush_impl: false
    1: quick-lint-js crashed. Please report this bug here:
    1: https://quick-lint-js.com/crash-report/
    1/2 Test #1: quick-lint-js-test ...............***Failed    9.77 sec

Add some debug logs to help isolate the bug.

[1] #859
@strager
Copy link
Collaborator Author

strager commented Oct 14, 2022

More info: https://github.com/quick-lint/quick-lint-js/actions/runs/3233883823/jobs/5330148207

1: fatal: file_output_stream::flush_impl failed to write data: The handle is invalid.
1: D:\a\quick-lint-js\quick-lint-js\src\quick-lint-js\io\output-stream.cpp:106: internal check failed in flush_impl: false

@strager
Copy link
Collaborator Author

strager commented Oct 14, 2022

I'm 90% sure that file_output_stream::flush_impl is being called by the destructor for the stream static-storage-duration variable in file_output_stream::get_stderr. It's being called during process teardown, hence the crash happening after all tests pass.

this->file_ cannot be an invalid handle (NULL or INVALID_HANDLE_VALUE). If it was, the assertion at the beginning of windows_handle_file_ref::write_full would have failed.

My only hypothesis is that the stderr standard handle got closed during process teardown. I don't know why this would happen. Maybe Microsoft's crt does this? But why?

I know two ways to accidentally fix this bug (but not fix the root cause):

  1. Make output_stream::flush not call flush_impl if there is no data to write, or
  2. Make parse_and_lint call file_output_stream::get_stderr only if options.print_parser_visits is true.

@strager
Copy link
Collaborator Author

strager commented Oct 15, 2022

Fixed in commit d910d71.

@strager strager closed this as completed Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false positive Severe bug: quick-lint-js rejects valid code
Projects
None yet
Development

No branches or pull requests

1 participant