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

src: make WaitForInspectorDisconnect an exit hook · nodejs/node@fc9e708 · GitHub

/ node Public

Commit fc9e708

Browse files
addaleax
authored and
MylesBorins
committed
src: make WaitForInspectorDisconnect an exit hook
Run inspector cleanup code on Environment teardown. This is part of a series of changes to make embedding easier, by requiring fewer internal methods to build a fully functioning Node.js instance. PR-URL: #30229 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
1 parent d14d9e8 commit fc9e708

6 files changed

Lines changed: 26 additions & 47 deletions

File tree

‎src/inspector_agent.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,13 @@ bool Agent::Start(const std::string& path,
778778
StartDebugSignalHandler();
779779
}
780780

781+
AtExit(parent_env_, [](void* env) {
782+
Agent* agent = static_cast<Environment*>(env)->inspector_agent();
783+
if (agent->IsActive()) {
784+
agent->WaitForDisconnect();
785+
}
786+
}, parent_env_);
787+
781788
bool wait_for_connect = options.wait_for_connect();
782789
if (parent_handle_) {
783790
wait_for_connect = parent_handle_->WaitForConnect();

‎src/node.cc‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -151,31 +151,6 @@ bool v8_is_profiling = false;
151151
struct V8Platform v8_platform;
152152
} // namespace per_process
153153

154-
#ifdef __POSIX__
155-
static const unsigned kMaxSignal = 32;
156-
#endif
157-
158-
void WaitForInspectorDisconnect(Environment* env) {
159-
#if HAVE_INSPECTOR
160-
161-
if (env->inspector_agent()->IsActive()) {
162-
// Restore signal dispositions, the app is done and is no longer
163-
// capable of handling signals.
164-
#if defined(__POSIX__) && !defined(NODE_SHARED_MODE)
165-
struct sigaction act;
166-
memset(&act, 0, sizeof(act));
167-
for (unsigned nr = 1; nr < kMaxSignal; nr += 1) {
168-
if (nr == SIGKILL || nr == SIGSTOP || nr == SIGPROF)
169-
continue;
170-
act.sa_handler = (nr == SIGPIPE) ? SIG_IGN : SIG_DFL;
171-
CHECK_EQ(0, sigaction(nr, &act, nullptr));
172-
}
173-
#endif
174-
env->inspector_agent()->WaitForDisconnect();
175-
}
176-
#endif
177-
}
178-
179154
void SignalExit(int signo) {
180155
ResetStdio();
181156
#ifdef __FreeBSD__
@@ -522,6 +497,7 @@ inline void PlatformInit() {
522497
CHECK_EQ(err, 0);
523498
#endif // HAVE_INSPECTOR
524499

500+
// TODO(addaleax): NODE_SHARED_MODE does not really make sense here.
525501
#ifndef NODE_SHARED_MODE
526502
// Restore signal dispositions, the parent process may have changed them.
527503
struct sigaction act;

‎src/node_internals.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ void PrintCaughtException(v8::Isolate* isolate,
9090
v8::Local<v8::Context> context,
9191
const v8::TryCatch& try_catch);
9292

93-
void WaitForInspectorDisconnect(Environment* env);
9493
void ResetStdio(); // Safe to call more than once and from signal handlers.
9594
void SignalExit(int signo);
9695
#ifdef __POSIX__
@@ -313,6 +312,10 @@ void StartProfilers(Environment* env);
313312
}
314313
#endif // HAVE_INSPECTOR
315314

315+
#ifdef __POSIX__
316+
static constexpr unsigned kMaxSignal = 32;
317+
#endif
318+
316319
bool HasSignalJSHandler(int signum);
317320

318321
#ifdef _WIN32

‎src/node_main_instance.cc‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,26 @@ int NodeMainInstance::Run() {
145145

146146
env->set_trace_sync_io(false);
147147
exit_code = EmitExit(env.get());
148-
WaitForInspectorDisconnect(env.get());
149148
}
150149

151150
env->set_can_call_into_js(false);
152151
env->stop_sub_worker_contexts();
153152
ResetStdio();
154153
env->RunCleanup();
154+
155+
// TODO(addaleax): Neither NODE_SHARED_MODE nor HAVE_INSPECTOR really
156+
// make sense here.
157+
#if HAVE_INSPECTOR && defined(__POSIX__) && !defined(NODE_SHARED_MODE)
158+
struct sigaction act;
159+
memset(&act, 0, sizeof(act));
160+
for (unsigned nr = 1; nr < kMaxSignal; nr += 1) {
161+
if (nr == SIGKILL || nr == SIGSTOP || nr == SIGPROF)
162+
continue;
163+
act.sa_handler = (nr == SIGPIPE) ? SIG_IGN : SIG_DFL;
164+
CHECK_EQ(0, sigaction(nr, &act, nullptr));
165+
}
166+
#endif
167+
155168
RunAtExit(env.get());
156169

157170
per_process::v8_platform.DrainVMTasks(isolate_);

‎src/node_process_methods.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,6 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args) {
426426
static void ReallyExit(const FunctionCallbackInfo<Value>& args) {
427427
Environment* env = Environment::GetCurrent(args);
428428
RunAtExit(env);
429-
WaitForInspectorDisconnect(env);
430429
int code = args[0]->Int32Value(env->context()).FromMaybe(0);
431430
env->Exit(code);
432431
}

‎src/node_worker.cc‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,6 @@ using v8::Value;
3737
namespace node {
3838
namespace worker {
3939

40-
namespace {
41-
42-
#if HAVE_INSPECTOR
43-
void WaitForWorkerInspectorToStop(Environment* child) {
44-
child->inspector_agent()->WaitForDisconnect();
45-
child->inspector_agent()->Stop();
46-
}
47-
#endif
48-
49-
} // anonymous namespace
50-
5140
Worker::Worker(Environment* env,
5241
Local<Object> wrap,
5342
const std::string& url,
@@ -191,9 +180,6 @@ void Worker::Run() {
191180
Locker locker(isolate_);
192181
Isolate::Scope isolate_scope(isolate_);
193182
SealHandleScope outer_seal(isolate_);
194-
#if HAVE_INSPECTOR
195-
bool inspector_started = false;
196-
#endif
197183

198184
DeleteFnPtr<Environment, FreeEnvironment> env_;
199185
OnScopeLeave cleanup_env([&]() {
@@ -223,10 +209,6 @@ void Worker::Run() {
223209
env_->stop_sub_worker_contexts();
224210
env_->RunCleanup();
225211
RunAtExit(env_.get());
226-
#if HAVE_INSPECTOR
227-
if (inspector_started)
228-
WaitForWorkerInspectorToStop(env_.get());
229-
#endif
230212

231213
// This call needs to be made while the `Environment` is still alive
232214
// because we assume that it is available for async tracking in the
@@ -270,7 +252,6 @@ void Worker::Run() {
270252
env_->InitializeDiagnostics();
271253
#if HAVE_INSPECTOR
272254
env_->InitializeInspector(inspector_parent_handle_.release());
273-
inspector_started = true;
274255
#endif
275256
HandleScope handle_scope(isolate_);
276257
AsyncCallbackScope callback_scope(env_.get());

0 commit comments

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