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

WIP Fix flakyness by hiding error #5193

Closed
wants to merge 8 commits into from

Conversation

gewoonwoutje
Copy link
Contributor

@gewoonwoutje gewoonwoutje commented May 9, 2020

Related to issue #3473 .

On my PC it fails consistently. The run command always exits with code -1073741819. There are 2 causes for problems:

  1. Deno exits before the async values are returned to js (i.e. response is not awaited).
  2. Deno exits js with a non-zero code implicitly, while no errors are thrown in js.

This PR is mainly meant as an indication where the flakyness comes from.

@@ -47,7 +47,7 @@ pub fn op_test_async(
}
let (tx, rx) = futures::channel::oneshot::channel::<Result<(), ()>>();
std::thread::spawn(move || {
std::thread::sleep(std::time::Duration::from_secs(1));
std::thread::sleep(std::time::Duration::from_millis(10));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This second is probably to indicate something slow as an example code thing, but it is also slowing down tests unnecessarily. Maybe remove it entirely?

throw new Error("Expected null response!");
}
return new Promise((res, rej) => {
Deno.core.setAsyncHandler(testAsync, (response) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if this is the kind of thing that should be set only once, but this enables that the async test can be awaited.

}
}

runTestSync();
runTestAsync();
await runTestAsync();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awaiting result to know for sure the expected lines are printed out.


runTestOpCount();
runTestPluginClose();

Deno.exit(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this makes deno run return a non-zero exit code implicitly. I have no idea where this implicit exit code is coming from...

Copy link
Contributor Author

@gewoonwoutje gewoonwoutje May 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this line gives:

~\deno\test_plugin [3473-flaky-plugin ≡ +0 ~1 -0 !]> $env:RUST_BACKTRACE="full"; cargo test
    Finished test [unoptimized + debuginfo] target(s) in 0.52s
     Running C:\Users\Wout\deno\target\debug\deps\test_plugin-df981ae4f791f766.exe

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running C:\Users\Wout\deno\target\debug\deps\integration_tests-f0601f9e1cabf4d4.exe

running 1 test
test basic ... FAILED

failures:

---- basic stdout ----
target_dir C:\Users\Wout\deno\target\debug
target_dir C:\Users\Wout\deno\target\debug
Deno run command exited with unsuccessful status exit code: 0xc0000005
stdout Hello from plugin. data: test | zero_copy: test
Plugin Sync Response: test
Hello from plugin. data: test | zero_copy: test
Plugin Async Response: test

stderr
thread 'basic' panicked at 'assertion failed: output.status.success()', test_plugin\tests\integration_tests.rs:47:3
stack backtrace:
   0:     0x7ff73874b11f - backtrace::backtrace::trace_unsynchronized
                               at C:\Users\VssAdministrator\.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.44\src\backtrace\mod.rs:66
   1:     0x7ff73874b11f - std::sys_common::backtrace::_print_fmt
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\sys_common\backtrace.rs:78
   2:     0x7ff73874b11f - std::sys_common::backtrace::_print::{{impl}}::fmt
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\sys_common\backtrace.rs:59
   3:     0x7ff73876615b - core::fmt::write
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libcore\fmt\mod.rs:1063
   4:     0x7ff738582f7c - std::io::Write::write_fmt<test::helpers::sink::Sink>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\io\mod.rs:1426
   5:     0x7ff7387440c8 - std::io::impls::{{impl}}::write_fmt<Write>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\io\impls.rs:156
   6:     0x7ff73874e76c - std::sys_common::backtrace::_print
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\sys_common\backtrace.rs:62
   7:     0x7ff73874e76c - std::sys_common::backtrace::print
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\sys_common\backtrace.rs:49
   8:     0x7ff73874e76c - std::panicking::default_hook::{{closure}}
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\panicking.rs:204
   9:     0x7ff73874e35e - std::panicking::default_hook
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\panicking.rs:221
  10:     0x7ff73874ef47 - std::panicking::rust_panic_with_hook
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\panicking.rs:470
  11:     0x7ff7387390e9 - std::panicking::begin_panic<str*>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\panicking.rs:397
  12:     0x7ff7385817ff - integration_tests::basic
                               at C:\Users\Wout\deno\test_plugin\tests\integration_tests.rs:47
  13:     0x7ff73858100e - integration_tests::basic::{{closure}}
                               at C:\Users\Wout\deno\test_plugin\tests\integration_tests.rs:20
  14:     0x7ff73858105b - core::ops::function::FnOnce::call_once<closure-0,()>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libcore\ops\function.rs:232
  15:     0x7ff738592317 - alloc::boxed::{{impl}}::call_once<(),FnOnce<()>>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\liballoc\boxed.rs:1017
  16:     0x7ff7387586a2 - panic_unwind::__rust_maybe_catch_panic
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libpanic_unwind\lib.rs:86
  17:     0x7ff7385b404a - std::panicking::try
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\panicking.rs:281
  18:     0x7ff7385b404a - std::panic::catch_unwind
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\panic.rs:394
  19:     0x7ff7385b404a - test::run_test_in_process
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libtest\lib.rs:542
  20:     0x7ff7385b404a - test::run_test::run_test_inner::{{closure}}
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libtest\lib.rs:451
  21:     0x7ff738582636 - std::sys_common::backtrace::__rust_begin_short_backtrace<closure-0,()>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\sys_common\backtrace.rs:130
  22:     0x7ff7385889e6 - std::thread::{{impl}}::spawn_unchecked::{{closure}}::{{closure}}
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\thread\mod.rs:475
  23:     0x7ff7385889e6 - std::panic::{{impl}}::call_once
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\panic.rs:318
  24:     0x7ff7385889e6 - std::panicking::try::do_call<std::panic::AssertUnwindSafe<closure-0>,()>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\panicking.rs:303
  25:     0x7ff7387586a2 - panic_unwind::__rust_maybe_catch_panic
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libpanic_unwind\lib.rs:86
  26:     0x7ff7385894e5 - std::panicking::try
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\panicking.rs:281
  27:     0x7ff7385894e5 - std::panic::catch_unwind
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\panic.rs:394
  28:     0x7ff7385894e5 - std::thread::{{impl}}::spawn_unchecked::{{closure}}
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libstd\thread\mod.rs:474
  29:     0x7ff7385894e5 - core::ops::function::FnOnce::call_once<closure-0,()>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\libcore\ops\function.rs:232
  30:     0x7ff73873f757 - alloc::boxed::{{impl}}::call_once<(),FnOnce<()>>
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\liballoc\boxed.rs:1017
  31:     0x7ff73875730c - alloc::boxed::{{impl}}::call_once
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\src\liballoc\boxed.rs:1017
  32:     0x7ff73875730c - std::sys_common::thread::start_thread
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\sys_common\thread.rs:13
  33:     0x7ff73875730c - std::sys::windows::thread::{{impl}}::new::thread_start
                               at /rustc/4fb7144ed159f94491249e86d5bbd033b5d60550\/src\libstd\sys\windows\thread.rs:51
  34:     0x7ffe01c37bd4 - BaseThreadInitThunk
  35:     0x7ffe0248ce51 - RtlUserThreadStart

@piscisaureus
Copy link
Member

-1073741819 is equivalent to 0xC0000005 which is the ntstatus code for STATUS_ACCESS_VIOLATION.
In other words, Deno is crashing (segfaulting) before it exits.

@gewoonwoutje
Copy link
Contributor Author

Ah, I did not know that!
So conclusion: not returning with Deno.exit() explicitly crashes Deno when using a plugin on windows... That's a problem😅. How can I help?

@gewoonwoutje
Copy link
Contributor Author

gewoonwoutje commented May 11, 2020

@piscisaureus Was this PR helpful? I'm kind of new to open sourcing so any feedback of how I can be more helpful is much appreciated! Alvast bedankt 😛

@piscisaureus
Copy link
Member

piscisaureus commented May 11, 2020

@gewoonwoutje

We always appreciate it when you try to hunt down bugs or flaky tests. So I'd encourage you to keep doing that.

I think in this particular case you got a little off track because of your preoccupation with the "implicit exit code" hypothesis. Had you run the test in a debugger, you might have found out about the segfault pretty early on.

That said, I had many WTF moments myself while debugging this issue and I've been at it for a few years, so I wouldn't feel too bad about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants