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

worker core dumped panic #12658

Closed
MierenManz opened this issue Nov 5, 2021 · 3 comments · Fixed by #12831
Closed

worker core dumped panic #12658

MierenManz opened this issue Nov 5, 2021 · 3 comments · Fixed by #12831
Labels
bug Something isn't working correctly runtime Relates to code in the runtime crate

Comments

@MierenManz
Copy link

const b64 = btoa(
  'await new Promise((res) => setTimeout(res, 1000));throw "error";',
);

const w = new Worker(`data:application/javascript;base64,${b64}`, {
  type: "module",
});

w.onerror = (e) => console.log("some error occured");
w.terminate();

This code has a chance to "panic" with a core dumped error on linux.
Both on deno 1.15.3 and canary(1.15.3+26a5471)

C stacktrace

#
# Fatal error in , line 0
# Check failed: (location_) != nullptr.
#
#
#
#FailureMessage Object: 0x7f620b7da3b0
==== C stack trace ===============================

    deno(+0x1b44ef3) [0x55aa66f9eef3]
    deno(+0x1b3ff6b) [0x55aa66f99f6b]
    deno(+0x1b40bb5) [0x55aa66f9abb5]
    deno(+0x1fc7385) [0x55aa67421385]
    deno(+0x1fc6be1) [0x55aa67420be1]
    deno(+0x1fc666d) [0x55aa6742066d]
    deno(+0x1fc64bc) [0x55aa674204bc]
    deno(+0x1b966ac) [0x55aa66ff06ac]
    deno(+0x35cfdb2) [0x55aa68a29db2]
    deno(+0x3090858) [0x55aa684ea858]
    deno(+0x2f3489c) [0x55aa6838e89c]
    deno(+0x2f3e4c2) [0x55aa683984c2]
    deno(+0x2954703) [0x55aa67dae703]
    /lib/x86_64-linux-gnu/libpthread.so.0(+0x9450) [0x7f6213336450]
    /lib/x86_64-linux-gnu/libc.so.6(clone+0x43) [0x7f621310ad53]
Trace/breakpoint trap (core dumped)
@lucacasonato lucacasonato added bug Something isn't working correctly runtime Relates to code in the runtime crate labels Nov 7, 2021
@andreubotella
Copy link
Contributor

andreubotella commented Nov 9, 2021

Although it's very rare, this code can also yield a Rust panic:

thread 'worker-0' panicked at 'assertion failed: status == v8::ModuleStatus::Errored', core/runtime.rs:1127:7
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

See #12713.

@andreubotella
Copy link
Contributor

Filed as a V8 bug: https://bugs.chromium.org/p/v8/issues/detail?id=12379

@andreubotella
Copy link
Contributor

andreubotella commented Nov 19, 2021

I've been looking into why the V8 bug doesn't crash Chrome, and it seems like what they do is, rather than terminating the worker's instance immediately, they:

  1. Signal to the worker thread that any long-running synchronous APIs must terminate. (I'm not sure exactly how this happens, or whether it can be replicated in Deno. Is this a thing we should care about?)
  2. Send a message to the worker's thread asking it to terminate.
  3. After a 2-second timeout, post a task on the current thread that will terminate the worker's isolate if it hasn't already.

Since v8::Module::evaluate is a synchronous function, the only case where this bug would be triggered in Chrome's model is if a worker's main module has TLA and has an endless loop before the first await point. A few cursory tests confirm that Chrome does indeed crash in this case.

Should we consider following the same model?

andreubotella pushed a commit to andreubotella/deno that referenced this issue Nov 21, 2021
Due to a bug in V8, terminating an isolate while a module with top-level
await is being evaluated would crash the process. This change makes it
so calling `worker.terminate()` will signal the worker to terminate at
the next iteration of the event loop, and it schedules a proper
termination of the worker's isolate after 2 seconds.

Closes denoland#12658
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly runtime Relates to code in the runtime crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants