Skip to content

src: remove invalid ToLocalChecked in EmitBeforeExit #35484

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.

Sign up for GitHub

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

Closed
addaleax wants to merge 1 commit into from
Closed

src: remove invalid ToLocalChecked in EmitBeforeExit #35484

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
src: remove invalid ToLocalChecked in EmitBeforeExit 
This call can fail, we should not use `.ToLocalChecked()` here.

In the long run, we should use variants of `EmitExit()` and
`EmitBeforeExit()` that avoid this problem by properly propagating
empty `MaybeLocal`s.

Example failure:

    21:07:17 not ok 2564 parallel/test-worker-terminate-source-map
    21:07:17   ---
    21:07:17   duration_ms: 0.385
    21:07:17   severity: crashed
    21:07:17   exitcode: -6
    21:07:17   stack: |-
    21:07:17     FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.
    21:07:17      1: 0x1012f04a5 node::Abort() (.cold.1) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
    21:07:17      2: 0x1000b52d9 node::Abort() [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
    21:07:17      3: 0x1000b543f node::OnFatalError(char const*, char const*) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
    21:07:17      4: 0x1001ffd70 v8::V8::ToLocalEmpty() [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
    21:07:17      5: 0x1000079b8 node::EmitBeforeExit(node::Environment*) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
    21:07:17      6: 0x100156fea node::worker::Worker::Run() [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
    21:07:17      7: 0x10015a4d2 node::worker::Worker::StartThread(v8::FunctionCallbackInfo<v8::Value> const&)::$_3::__invoke(void*) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
    21:07:17      8: 0x7fff72fe7109 _pthread_start [/usr/lib/system/libsystem_pthread.dylib]
    21:07:17      9: 0x7fff72fe2b8b thread_start [/usr/lib/system/libsystem_pthread.dylib]
    21:07:17   ...
  • Loading branch information
@addaleax
addaleax committed Oct 3, 2020
commit b82666716b21bdce299ce432f9abec493c52910b
4 changes: 3 additions & 1 deletion src/api/hooks.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ void EmitBeforeExit(Environment* env) {
Local<Integer> exit_code;
if (!exit_code_v->ToInteger(env->context()).ToLocal(&exit_code)) return;

ProcessEmit(env, "beforeExit", exit_code).ToLocalChecked();
// TODO(addaleax): Provide variants of EmitExit() and EmitBeforeExit() that
// actually forward empty MaybeLocal<>s (and check env->can_call_into_js()).
USE(ProcessEmit(env, "beforeExit", exit_code));
}

int EmitExit(Environment* env) {
Expand Down