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

Assertion failure when calling process.exit() from a Worker that has TLA #43182

Closed
aduh95 opened this issue May 22, 2022 · 10 comments
Closed

Assertion failure when calling process.exit() from a Worker that has TLA #43182

aduh95 opened this issue May 22, 2022 · 10 comments
Labels
confirmed-bug Issues with confirmed bugs. worker Issues and PRs related to Worker support.

Comments

@aduh95
Copy link
Contributor

aduh95 commented May 22, 2022

Version

master

Platform

macOS

Subsystem

worker_threads

What steps will reproduce the bug?

new worker_threads.Worker(new URL("data:text/javascript,process.exit(0);await new Promise(()=>{})"))

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

No crash

What do you see instead?

$ out/Release/node -e 'new worker_threads.Worker(new URL("data:text/javascript,process.exit(0);await new Promise(()=>{})"))'


#
# Fatal error in , line 0
# Check failed: (location_) != nullptr.
#
#
#
#FailureMessage Object: 0x700008060840
 1: 0x108604622 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() […/out/Release/node]
 2: 0x109772823 V8_Fatal(char const*, ...) […/out/Release/node]
 3: 0x108be671c v8::internal::SourceTextModule::ExecuteAsyncModule(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SourceTextModule>) […/out/Release/node]
 4: 0x108be5f35 v8::internal::SourceTextModule::InnerModuleEvaluation(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SourceTextModule>, v8::internal::ZoneForwardList<v8::internal::Handle<v8::internal::SourceTextModule> >*, unsigned int*) […/out/Release/node]
 5: 0x108be5d6b v8::internal::SourceTextModule::InnerModuleEvaluation(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SourceTextModule>, v8::internal::ZoneForwardList<v8::internal::Handle<v8::internal::SourceTextModule> >*, unsigned int*) […/out/Release/node]
 6: 0x108be583b v8::internal::SourceTextModule::Evaluate(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SourceTextModule>) […/out/Release/node]
 7: 0x10871b43c v8::Module::Evaluate(v8::Local<v8::Context>) […/out/Release/node]
 8: 0x10855c6ae node::loader::ModuleWrap::Evaluate(v8::FunctionCallbackInfo<v8::Value> const&) […/out/Release/node]
 9: 0x10877c9f8 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) […/out/Release/node]
10: 0x10877c54b v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) […/out/Release/node]
11: 0x10877bc2b v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) […/out/Release/node]
12: 0x1090d2b39 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit […/out/Release/node]
13: 0x109056a90 Builtins_InterpreterEntryTrampoline […/out/Release/node]
[1]    43848 trace trap  out/Release/node -e 

Additional information

The bug is reproducable on v14.x, v16.x, and v18.x lines.

@aduh95
Copy link
Contributor Author

aduh95 commented May 22, 2022

/cc @nodejs/workers

@aduh95 aduh95 added confirmed-bug Issues with confirmed bugs. worker Issues and PRs related to Worker support. labels May 22, 2022
@legendecas
Copy link
Member

V8 asserts the evaluation of the module's async function to succeed without exception. However, the problem is that TerminateExecution initiated by process.exit is breaking that assumption. I'll work on this and submit a fix to v8.

@sajal50
Copy link
Contributor

sajal50 commented Jun 16, 2022

@legendecas, are you working on this?

@legendecas
Copy link
Member

@sajal50 yeah, it is still under review at https://chromium-review.googlesource.com/c/v8/v8/+/3696493.

@legendecas
Copy link
Member

https://chromium-review.googlesource.com/c/v8/v8/+/3696493 has landed. I'll continue to investigate if node handles these termination exceptions properly.

@aduh95
Copy link
Contributor Author

aduh95 commented Jul 9, 2022

@legendecas I've tried to cherry-pick your commit in node repo, but it looks like it relies on V8 APIs that are not available in the version of V8 we are using. Do you know if it's worth a backport or if we'd be better off waiting for V8 10.5 to stabilize?

legendecas added a commit to legendecas/node that referenced this issue Jul 10, 2022
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    SourceTextModule::ExecuteAsyncModule asserts the execution of
    the module's async function to succeed without exception. However,
    the problem is that TerminateExecution initiated by embedders is
    breaking that assumption. The execution can be terminated with an
    exception and the exception is not catchable by JavaScript.

    The uncatchable exceptions during the async module evaluation need
    to be raised to the embedder and not crash the process if possible.

    Refs: nodejs#43182

    Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2
@legendecas
Copy link
Member

@aduh95 thanks for the ping. I think the patch should be straightforward to be backported. Submitted #43751 and added a test case for the issue.

legendecas added a commit to legendecas/node that referenced this issue Jul 12, 2022
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    SourceTextModule::ExecuteAsyncModule asserts the execution of
    the module's async function to succeed without exception. However,
    the problem is that TerminateExecution initiated by embedders is
    breaking that assumption. The execution can be terminated with an
    exception and the exception is not catchable by JavaScript.

    The uncatchable exceptions during the async module evaluation need
    to be raised to the embedder and not crash the process if possible.

    Refs: nodejs#43182

    Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2
legendecas added a commit that referenced this issue Jul 13, 2022
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    SourceTextModule::ExecuteAsyncModule asserts the execution of
    the module's async function to succeed without exception. However,
    the problem is that TerminateExecution initiated by embedders is
    breaking that assumption. The execution can be terminated with an
    exception and the exception is not catchable by JavaScript.

    The uncatchable exceptions during the async module evaluation need
    to be raised to the embedder and not crash the process if possible.

    Refs: #43182

    Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2

PR-URL: #43751
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
danielleadams pushed a commit that referenced this issue Jul 26, 2022
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    SourceTextModule::ExecuteAsyncModule asserts the execution of
    the module's async function to succeed without exception. However,
    the problem is that TerminateExecution initiated by embedders is
    breaking that assumption. The execution can be terminated with an
    exception and the exception is not catchable by JavaScript.

    The uncatchable exceptions during the async module evaluation need
    to be raised to the embedder and not crash the process if possible.

    Refs: #43182

    Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2

PR-URL: #43751
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
targos pushed a commit that referenced this issue Aug 2, 2022
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    SourceTextModule::ExecuteAsyncModule asserts the execution of
    the module's async function to succeed without exception. However,
    the problem is that TerminateExecution initiated by embedders is
    breaking that assumption. The execution can be terminated with an
    exception and the exception is not catchable by JavaScript.

    The uncatchable exceptions during the async module evaluation need
    to be raised to the embedder and not crash the process if possible.

    Refs: #43182

    Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2
PR-URL: #43751
Backport-PR-URL: #44085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
@szmarczak
Copy link
Member

Another reproduction case (it's weird, remove join and it works?):

import { join } from 'node:path';
import { isMainThread, Worker } from 'node:worker_threads';

if (isMainThread) {
    new Worker(new URL(import.meta.url));
    await 0;
} else {
    process.exit();
    join();
}

Is there a chance for this to be backported into Node.js 14?

guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    SourceTextModule::ExecuteAsyncModule asserts the execution of
    the module's async function to succeed without exception. However,
    the problem is that TerminateExecution initiated by embedders is
    breaking that assumption. The execution can be terminated with an
    exception and the exception is not catchable by JavaScript.

    The uncatchable exceptions during the async module evaluation need
    to be raised to the embedder and not crash the process if possible.

    Refs: nodejs/node#43182

    Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2
PR-URL: nodejs/node#43751
Backport-PR-URL: nodejs/node#44085
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
@legendecas
Copy link
Member

legendecas commented Oct 26, 2022

@szmarczak I tried a backport but V8 has changed a lot since v14. It might need a totally new patch for the issue on v14 (and a refresh review on the patch too).

@szmarczak
Copy link
Member

No problem. Thanks for trying ❤️

abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    SourceTextModule::ExecuteAsyncModule asserts the execution of
    the module's async function to succeed without exception. However,
    the problem is that TerminateExecution initiated by embedders is
    breaking that assumption. The execution can be terminated with an
    exception and the exception is not catchable by JavaScript.

    The uncatchable exceptions during the async module evaluation need
    to be raised to the embedder and not crash the process if possible.

    Refs: nodejs/node#43182

    Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2

PR-URL: nodejs/node#43751
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants