FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: add maybe versions of EmitExit and EmitBeforeExit · nodejs/node@f421422 · GitHub

/ node Public

Commit f421422

Browse files
addaleax
authored and
MylesBorins
committed
src: add maybe versions of EmitExit and EmitBeforeExit
This addresses a TODO comment, and removes invalid `.ToLocalChecked()` calls from our code base. PR-URL: #35486 Backport-PR-URL: #38786 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 7031eae commit f421422

8 files changed

Lines changed: 105 additions & 32 deletions

File tree

‎doc/api/embedding.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,18 +181,19 @@ int RunNodeInstance(MultiIsolatePlatform* platform,
181181
more = uv_loop_alive(&loop);
182182
if (more) continue;
183183
184-
// node::EmitBeforeExit() is used to emit the 'beforeExit' event on
185-
// the `process` object.
186-
node::EmitBeforeExit(env.get());
184+
// node::EmitProcessBeforeExit() is used to emit the 'beforeExit' event
185+
// on the `process` object.
186+
if (node::EmitProcessBeforeExit(env.get()).IsNothing())
187+
break;
187188
188189
// 'beforeExit' can also schedule new work that keeps the event loop
189190
// running.
190191
more = uv_loop_alive(&loop);
191192
} while (more == true);
192193
}
193194
194-
// node::EmitExit() returns the current exit code.
195-
exit_code = node::EmitExit(env.get());
195+
// node::EmitProcessExit() returns the current exit code.
196+
exit_code = node::EmitProcessExit(env.get()).FromMaybe(1);
196197
197198
// node::Stop() can be used to explicitly stop the event loop and keep
198199
// further JavaScript from running. It can be called from any thread,

‎src/api/hooks.cc‎

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@ using v8::Context;
99
using v8::HandleScope;
1010
using v8::Integer;
1111
using v8::Isolate;
12+
using v8::Just;
1213
using v8::Local;
14+
using v8::Maybe;
1315
using v8::NewStringType;
16+
using v8::Nothing;
1417
using v8::Object;
1518
using v8::String;
1619
using v8::Value;
@@ -30,6 +33,10 @@ void AtExit(Environment* env, void (*cb)(void* arg), void* arg) {
3033
}
3134

3235
void EmitBeforeExit(Environment* env) {
36+
USE(EmitProcessBeforeExit(env));
37+
}
38+
39+
Maybe<bool> EmitProcessBeforeExit(Environment* env) {
3340
TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
3441
"BeforeExit", env);
3542
if (!env->destroy_async_id_list()->empty())
@@ -40,39 +47,49 @@ void EmitBeforeExit(Environment* env) {
4047

4148
Local<Value> exit_code_v;
4249
if (!env->process_object()->Get(env->context(), env->exit_code_string())
43-
.ToLocal(&exit_code_v)) return;
50+
.ToLocal(&exit_code_v)) return Nothing<bool>();
4451

4552
Local<Integer> exit_code;
46-
if (!exit_code_v->ToInteger(env->context()).ToLocal(&exit_code)) return;
53+
if (!exit_code_v->ToInteger(env->context()).ToLocal(&exit_code)) {
54+
return Nothing<bool>();
55+
}
4756

48-
// TODO(addaleax): Provide variants of EmitExit() and EmitBeforeExit() that
49-
// actually forward empty MaybeLocal<>s (and check env->can_call_into_js()).
50-
USE(ProcessEmit(env, "beforeExit", exit_code));
57+
return ProcessEmit(env, "beforeExit", exit_code).IsEmpty() ?
58+
Nothing<bool>() : Just(true);
5159
}
5260

5361
int EmitExit(Environment* env) {
62+
return EmitProcessExit(env).FromMaybe(1);
63+
}
64+
65+
Maybe<int> EmitProcessExit(Environment* env) {
5466
// process.emit('exit')
5567
HandleScope handle_scope(env->isolate());
5668
Context::Scope context_scope(env->context());
5769
Local<Object> process_object = env->process_object();
58-
process_object
70+
71+
// TODO(addaleax): It might be nice to share process._exiting and
72+
// process.exitCode via getter/setter pairs that pass data directly to the
73+
// native side, so that we don't manually have to read and write JS properties
74+
// here. These getters could use e.g. a typed array for performance.
75+
if (process_object
5976
->Set(env->context(),
6077
FIXED_ONE_BYTE_STRING(env->isolate(), "_exiting"),
61-
True(env->isolate()))
62-
.Check();
78+
True(env->isolate())).IsNothing()) return Nothing<int>();
6379

6480
Local<String> exit_code = env->exit_code_string();
65-
int code = process_object->Get(env->context(), exit_code)
66-
.ToLocalChecked()
67-
->Int32Value(env->context())
68-
.ToChecked();
69-
ProcessEmit(env, "exit", Integer::New(env->isolate(), code));
70-
71-
// Reload exit code, it may be changed by `emit('exit')`
72-
return process_object->Get(env->context(), exit_code)
73-
.ToLocalChecked()
74-
->Int32Value(env->context())
75-
.ToChecked();
81+
Local<Value> code_v;
82+
int code;
83+
if (!process_object->Get(env->context(), exit_code).ToLocal(&code_v) ||
84+
!code_v->Int32Value(env->context()).To(&code) ||
85+
ProcessEmit(env, "exit", Integer::New(env->isolate(), code)).IsEmpty() ||
86+
// Reload exit code, it may be changed by `emit('exit')`
87+
!process_object->Get(env->context(), exit_code).ToLocal(&code_v) ||
88+
!code_v->Int32Value(env->context()).To(&code)) {
89+
return Nothing<int>();
90+
}
91+
92+
return Just(code);
7693
}
7794

7895
typedef void (*CleanupHook)(void* arg);

‎src/node.h‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,19 @@ NODE_EXTERN void FreePlatform(MultiIsolatePlatform* platform);
539539
NODE_EXTERN v8::TracingController* GetTracingController();
540540
NODE_EXTERN void SetTracingController(v8::TracingController* controller);
541541

542-
NODE_EXTERN void EmitBeforeExit(Environment* env);
543-
NODE_EXTERN int EmitExit(Environment* env);
542+
// Run `process.emit('beforeExit')` as it would usually happen when Node.js is
543+
// run in standalone mode.
544+
NODE_EXTERN v8::Maybe<bool> EmitProcessBeforeExit(Environment* env);
545+
NODE_DEPRECATED("Use Maybe version (EmitProcessBeforeExit) instead",
546+
NODE_EXTERN void EmitBeforeExit(Environment* env));
547+
// Run `process.emit('exit')` as it would usually happen when Node.js is run
548+
// in standalone mode. The return value corresponds to the exit code.
549+
NODE_EXTERN v8::Maybe<int> EmitProcessExit(Environment* env);
550+
NODE_DEPRECATED("Use Maybe version (EmitProcessExit) instead",
551+
NODE_EXTERN int EmitExit(Environment* env));
552+
553+
// Runs hooks added through `AtExit()`. This is part of `FreeEnvironment()`,
554+
// so calling it manually is typically not necessary.
544555
NODE_EXTERN void RunAtExit(Environment* env);
545556

546557
// This may return nullptr if the current v8::Context is not associated

‎src/node_main_instance.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ int NodeMainInstance::Run() {
135135
if (more && !env->is_stopping()) continue;
136136

137137
if (!uv_loop_alive(env->event_loop())) {
138-
EmitBeforeExit(env.get());
138+
if (EmitProcessBeforeExit(env.get()).IsNothing())
139+
break;
139140
}
140141

141142
// Emit `beforeExit` if the loop became alive either after emitting
@@ -148,7 +149,7 @@ int NodeMainInstance::Run() {
148149

149150
env->set_trace_sync_io(false);
150151
if (!env->is_stopping()) env->VerifyNoStrongBaseObjects();
151-
exit_code = EmitExit(env.get());
152+
exit_code = EmitProcessExit(env.get()).FromMaybe(1);
152153
}
153154

154155
ResetStdio();

‎src/node_worker.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,8 @@ void Worker::Run() {
352352
more = uv_loop_alive(&data.loop_);
353353
if (more && !is_stopped()) continue;
354354

355-
EmitBeforeExit(env_.get());
355+
if (EmitProcessBeforeExit(env_.get()).IsNothing())
356+
break;
356357

357358
// Emit `beforeExit` if the loop became alive either after emitting
358359
// event, or after running some callbacks.
@@ -368,7 +369,7 @@ void Worker::Run() {
368369
bool stopped = is_stopped();
369370
if (!stopped) {
370371
env_->VerifyNoStrongBaseObjects();
371-
exit_code = EmitExit(env_.get());
372+
exit_code = EmitProcessExit(env_.get()).FromMaybe(1);
372373
}
373374
Mutex::ScopedLock lock(mutex_);
374375
if (exit_code_ == 0 && !stopped)

‎test/embedding/embedtest.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,14 @@ int RunNodeInstance(MultiIsolatePlatform* platform,
110110
more = uv_loop_alive(&loop);
111111
if (more) continue;
112112

113-
node::EmitBeforeExit(env.get());
113+
if (node::EmitProcessBeforeExit(env.get()).IsNothing())
114+
break;
115+
114116
more = uv_loop_alive(&loop);
115117
} while (more == true);
116118
}
117119

118-
exit_code = node::EmitExit(env.get());
120+
exit_code = node::EmitProcessExit(env.get()).FromMaybe(1);
119121

120122
node::Stop(env.get());
121123
}

‎test/parallel/test-process-beforeexit-throw-exit.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
const common = require('../common');
3+
common.skipIfWorker();
4+
5+
// Test that 'exit' is emitted if 'beforeExit' throws.
6+
7+
process.on('exit', common.mustCall(() => {
8+
process.exitCode = 0;
9+
}));
10+
process.on('beforeExit', common.mustCall(() => {
11+
throw new Error();
12+
}));

‎test/parallel/test-worker-beforeexit-throw-exit.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { Worker } = require('worker_threads');
5+
6+
// Test that 'exit' is emitted if 'beforeExit' throws, both inside the Worker.
7+
8+
const workerData = new Uint8Array(new SharedArrayBuffer(2));
9+
const w = new Worker(`
10+
const { workerData } = require('worker_threads');
11+
process.on('exit', () => {
12+
workerData[0] = 100;
13+
});
14+
process.on('beforeExit', () => {
15+
workerData[1] = 200;
16+
throw new Error('banana');
17+
});
18+
`, { eval: true, workerData });
19+
20+
w.on('error', common.mustCall((err) => {
21+
assert.strictEqual(err.message, 'banana');
22+
}));
23+
24+
w.on('exit', common.mustCall((code) => {
25+
assert.strictEqual(code, 1);
26+
assert.strictEqual(workerData[0], 100);
27+
assert.strictEqual(workerData[1], 200);
28+
}));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL