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

async_hooks,worker: fix process.exit() crashes in async fn with async_hook enabled #33347

Closed

Commits on May 10, 2020

  1. worker: call CancelTerminateExecution() before exiting Locker

    As the comment indicates, this fixes a DCHECK failure, although I don’t
    quite understand why it is happening in the first place.
    addaleax committed May 10, 2020
    Copy the full SHA
    b523a31 View commit details
    Browse the repository at this point in the history

Commits on May 11, 2020

  1. async_hooks: clear async_id_stack for terminations in more places

    Termination exceptions are similar to uncaught exceptions in that they
    should clear the async id stack, because no ongoing async callbacks
    will be brought to completion when execution terminates.
    
    Previously, there was a check that made sure that that happened when
    the termination occurred during the callback itself, but no such check
    was in place for the case that the termination occurred during
    microtasks started by them. This commit adds such a check, both for
    microtasks and the `nextTick` queue. The latter addition doesn’t fix
    a crash, but still makes sense conceptually.
    
    The condition here is also flipped from applying only on Worker threads
    to also applying on the main thread, and setting the `failed_` flag
    rather than reading it. The former makes sense because the public C++
    `Stop(env)` API can have the same effect as worker thread termination,
    but on the main thread rather than a Worker thread.
    addaleax committed May 11, 2020
    Copy the full SHA
    d90c3d1 View commit details
    Browse the repository at this point in the history
  2. test: regression tests for async_hooks + Promise + Worker interaction

    Add regression tests for the case in which an async_hook is enabled
    inside a Worker thread and `process.exit()` is called during the
    async part of an async function.
    
    This commit includes multiple tests that seem like they should all
    crash in a similar way, but interestingly don’t. In particular, it’s
    surprising that the presence of a statement after `process.exit()`
    in a function has an effect on the kind of crash that’s being
    exhibited (V8 DCHECK vs. assertion in our own code) and the
    circumstances under which it crashes (e.g. the -1 and -2 tests
    can be “fixed” by reverting 13c5a16, although they
    should have the same behavior as the -3 and -4 tests).
    addaleax committed May 11, 2020
    Copy the full SHA
    7b45371 View commit details
    Browse the repository at this point in the history
  3. fixup! test: regression tests for async_hooks + Promise + Worker inte…

    …raction
    
    Co-authored-by: Colin Ihrig <cjihrig@gmail.com>
    addaleax and cjihrig committed May 11, 2020
    Copy the full SHA
    3d9d564 View commit details
    Browse the repository at this point in the history