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

src: add maybe versions of EmitExit and EmitBeforeExit #35486

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions doc/api/embedding.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,18 +181,19 @@ int RunNodeInstance(MultiIsolatePlatform* platform,
more = uv_loop_alive(&loop);
if (more) continue;

// node::EmitBeforeExit() is used to emit the 'beforeExit' event on
// the `process` object.
node::EmitBeforeExit(env.get());
// node::EmitProcessBeforeExit() is used to emit the 'beforeExit' event
// on the `process` object.
if (node::EmitProcessBeforeExit(env.get()).IsNothing())
break;

// 'beforeExit' can also schedule new work that keeps the event loop
// running.
more = uv_loop_alive(&loop);
} while (more == true);
}

// node::EmitExit() returns the current exit code.
exit_code = node::EmitExit(env.get());
// node::EmitProcessExit() returns the current exit code.
exit_code = node::EmitProcessExit(env.get()).FromMaybe(1);

// node::Stop() can be used to explicitly stop the event loop and keep
// further JavaScript from running. It can be called from any thread,
Expand Down
55 changes: 36 additions & 19 deletions src/api/hooks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ using v8::Context;
using v8::HandleScope;
using v8::Integer;
using v8::Isolate;
using v8::Just;
using v8::Local;
using v8::Maybe;
using v8::NewStringType;
using v8::Nothing;
using v8::Object;
using v8::String;
using v8::Value;
Expand All @@ -30,6 +33,10 @@ void AtExit(Environment* env, void (*cb)(void* arg), void* arg) {
}

void EmitBeforeExit(Environment* env) {
USE(EmitProcessBeforeExit(env));
}

Maybe<bool> EmitProcessBeforeExit(Environment* env) {
TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
"BeforeExit", env);
if (!env->destroy_async_id_list()->empty())
Expand All @@ -40,39 +47,49 @@ void EmitBeforeExit(Environment* env) {

Local<Value> exit_code_v;
if (!env->process_object()->Get(env->context(), env->exit_code_string())
.ToLocal(&exit_code_v)) return;
.ToLocal(&exit_code_v)) return Nothing<bool>();

Local<Integer> exit_code;
if (!exit_code_v->ToInteger(env->context()).ToLocal(&exit_code)) return;
if (!exit_code_v->ToInteger(env->context()).ToLocal(&exit_code)) {
return Nothing<bool>();
}

// 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));
return ProcessEmit(env, "beforeExit", exit_code).IsEmpty() ?
Nothing<bool>() : Just(true);
}

int EmitExit(Environment* env) {
return EmitProcessExit(env).FromMaybe(1);
}

Maybe<int> EmitProcessExit(Environment* env) {
// process.emit('exit')
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
Local<Object> process_object = env->process_object();
process_object

// TODO(addaleax): It might be nice to share process._exiting and
// process.exitCode via getter/setter pairs that pass data directly to the
// native side, so that we don't manually have to read and write JS properties
// here. These getters could use e.g. a typed array for performance.
if (process_object
->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "_exiting"),
True(env->isolate()))
.Check();
True(env->isolate())).IsNothing()) return Nothing<int>();

Local<String> exit_code = env->exit_code_string();
int code = process_object->Get(env->context(), exit_code)
.ToLocalChecked()
->Int32Value(env->context())
.ToChecked();
ProcessEmit(env, "exit", Integer::New(env->isolate(), code));

// Reload exit code, it may be changed by `emit('exit')`
return process_object->Get(env->context(), exit_code)
.ToLocalChecked()
->Int32Value(env->context())
.ToChecked();
Local<Value> code_v;
int code;
if (!process_object->Get(env->context(), exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(env->context()).To(&code) ||
ProcessEmit(env, "exit", Integer::New(env->isolate(), code)).IsEmpty() ||
// Reload exit code, it may be changed by `emit('exit')`
!process_object->Get(env->context(), exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(env->context()).To(&code)) {
return Nothing<int>();
}

return Just(code);
}

typedef void (*CleanupHook)(void* arg);
Expand Down
15 changes: 13 additions & 2 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,19 @@ NODE_EXTERN void FreePlatform(MultiIsolatePlatform* platform);
NODE_EXTERN v8::TracingController* GetTracingController();
NODE_EXTERN void SetTracingController(v8::TracingController* controller);

NODE_EXTERN void EmitBeforeExit(Environment* env);
NODE_EXTERN int EmitExit(Environment* env);
// Run `process.emit('beforeExit')` as it would usually happen when Node.js is
// run in standalone mode.
NODE_EXTERN v8::Maybe<bool> EmitProcessBeforeExit(Environment* env);
NODE_DEPRECATED("Use Maybe version (EmitProcessBeforeExit) instead",
NODE_EXTERN void EmitBeforeExit(Environment* env));
// Run `process.emit('exit')` as it would usually happen when Node.js is run
// in standalone mode. The return value corresponds to the exit code.
NODE_EXTERN v8::Maybe<int> EmitProcessExit(Environment* env);
NODE_DEPRECATED("Use Maybe version (EmitProcessExit) instead",
NODE_EXTERN int EmitExit(Environment* env));

// Runs hooks added through `AtExit()`. This is part of `FreeEnvironment()`,
// so calling it manually is typically not necessary.
NODE_EXTERN void RunAtExit(Environment* env);

// This may return nullptr if the current v8::Context is not associated
Expand Down
5 changes: 3 additions & 2 deletions src/node_main_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ int NodeMainInstance::Run(const EnvSerializeInfo* env_info) {
if (more && !env->is_stopping()) continue;

if (!uv_loop_alive(env->event_loop())) {
EmitBeforeExit(env.get());
if (EmitProcessBeforeExit(env.get()).IsNothing())
break;
}

// Emit `beforeExit` if the loop became alive either after emitting
Expand All @@ -169,7 +170,7 @@ int NodeMainInstance::Run(const EnvSerializeInfo* env_info) {

env->set_trace_sync_io(false);
if (!env->is_stopping()) env->VerifyNoStrongBaseObjects();
exit_code = EmitExit(env.get());
exit_code = EmitProcessExit(env.get()).FromMaybe(1);
}

ResetStdio();
Expand Down
5 changes: 3 additions & 2 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ void Worker::Run() {
more = uv_loop_alive(&data.loop_);
if (more && !is_stopped()) continue;

EmitBeforeExit(env_.get());
if (EmitProcessBeforeExit(env_.get()).IsNothing())
break;

// Emit `beforeExit` if the loop became alive either after emitting
// event, or after running some callbacks.
Expand All @@ -364,7 +365,7 @@ void Worker::Run() {
bool stopped = is_stopped();
if (!stopped) {
env_->VerifyNoStrongBaseObjects();
exit_code = EmitExit(env_.get());
exit_code = EmitProcessExit(env_.get()).FromMaybe(1);
}
Mutex::ScopedLock lock(mutex_);
if (exit_code_ == 0 && !stopped)
Expand Down
6 changes: 4 additions & 2 deletions test/embedding/embedtest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,14 @@ int RunNodeInstance(MultiIsolatePlatform* platform,
more = uv_loop_alive(&loop);
if (more) continue;

node::EmitBeforeExit(env.get());
if (node::EmitProcessBeforeExit(env.get()).IsNothing())
break;

more = uv_loop_alive(&loop);
} while (more == true);
}

exit_code = node::EmitExit(env.get());
exit_code = node::EmitProcessExit(env.get()).FromMaybe(1);

node::Stop(env.get());
}
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-process-beforeexit-throw-exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';
const common = require('../common');
common.skipIfWorker();

// Test that 'exit' is emitted if 'beforeExit' throws.

process.on('exit', common.mustCall(() => {
process.exitCode = 0;
}));
process.on('beforeExit', common.mustCall(() => {
throw new Error();
}));
28 changes: 28 additions & 0 deletions test/parallel/test-worker-beforeexit-throw-exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { Worker } = require('worker_threads');

// Test that 'exit' is emitted if 'beforeExit' throws, both inside the Worker.

const workerData = new Uint8Array(new SharedArrayBuffer(2));
const w = new Worker(`
const { workerData } = require('worker_threads');
process.on('exit', () => {
workerData[0] = 100;
});
process.on('beforeExit', () => {
workerData[1] = 200;
throw new Error('banana');
});
`, { eval: true, workerData });

w.on('error', common.mustCall((err) => {
assert.strictEqual(err.message, 'banana');
}));

w.on('exit', common.mustCall((code) => {
assert.strictEqual(code, 1);
assert.strictEqual(workerData[0], 100);
assert.strictEqual(workerData[1], 200);
}));