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

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

/ node Public

Commit db7deb6

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 cd233e3 commit db7deb6

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
@@ -162,31 +162,6 @@ bool v8_is_profiling = false;
162162
struct V8Platform v8_platform;
163163
} // namespace per_process
164164

165-
#ifdef __POSIX__
166-
static const unsigned kMaxSignal = 32;
167-
#endif
168-
169-
void WaitForInspectorDisconnect(Environment* env) {
170-
#if HAVE_INSPECTOR
171-
172-
if (env->inspector_agent()->IsActive()) {
173-
// Restore signal dispositions, the app is done and is no longer
174-
// capable of handling signals.
175-
#if defined(__POSIX__) && !defined(NODE_SHARED_MODE)
176-
struct sigaction act;
177-
memset(&act, 0, sizeof(act));
178-
for (unsigned nr = 1; nr < kMaxSignal; nr += 1) {
179-
if (nr == SIGKILL || nr == SIGSTOP || nr == SIGPROF)
180-
continue;
181-
act.sa_handler = (nr == SIGPIPE) ? SIG_IGN : SIG_DFL;
182-
CHECK_EQ(0, sigaction(nr, &act, nullptr));
183-
}
184-
#endif
185-
env->inspector_agent()->WaitForDisconnect();
186-
}
187-
#endif
188-
}
189-
190165
#ifdef __POSIX__
191166
void SignalExit(int signo, siginfo_t* info, void* ucontext) {
192167
ResetStdio();
@@ -553,6 +528,7 @@ inline void PlatformInit() {
553528
CHECK_EQ(err, 0);
554529
#endif // HAVE_INSPECTOR
555530

531+
// TODO(addaleax): NODE_SHARED_MODE does not really make sense here.
556532
#ifndef NODE_SHARED_MODE
557533
// Restore signal dispositions, the parent process may have changed them.
558534
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
#ifdef __POSIX__
9695
void SignalExit(int signal, siginfo_t* info, void* ucontext);
@@ -321,6 +320,10 @@ void StartProfilers(Environment* env);
321320
}
322321
#endif // HAVE_INSPECTOR
323322

323+
#ifdef __POSIX__
324+
static constexpr unsigned kMaxSignal = 32;
325+
#endif
326+
324327
bool HasSignalJSHandler(int signum);
325328

326329
#ifdef _WIN32

‎src/node_main_instance.cc‎

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

153153
env->set_trace_sync_io(false);
154154
exit_code = EmitExit(env.get());
155-
WaitForInspectorDisconnect(env.get());
156155
}
157156

158157
env->set_can_call_into_js(false);
159158
env->stop_sub_worker_contexts();
160159
ResetStdio();
161160
env->RunCleanup();
161+
162+
// TODO(addaleax): Neither NODE_SHARED_MODE nor HAVE_INSPECTOR really
163+
// make sense here.
164+
#if HAVE_INSPECTOR && 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+
162175
RunAtExit(env.get());
163176

164177
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
@@ -433,7 +433,6 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args) {
433433
static void ReallyExit(const FunctionCallbackInfo<Value>& args) {
434434
Environment* env = Environment::GetCurrent(args);
435435
RunAtExit(env);
436-
WaitForInspectorDisconnect(env);
437436
int code = args[0]->Int32Value(env->context()).FromMaybe(0);
438437
env->Exit(code);
439438
}

‎src/node_worker.cc‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,6 @@ using v8::Value;
4242
namespace node {
4343
namespace worker {
4444

45-
namespace {
46-
47-
#if HAVE_INSPECTOR
48-
void WaitForWorkerInspectorToStop(Environment* child) {
49-
child->inspector_agent()->WaitForDisconnect();
50-
child->inspector_agent()->Stop();
51-
}
52-
#endif
53-
54-
} // anonymous namespace
55-
5645
Worker::Worker(Environment* env,
5746
Local<Object> wrap,
5847
const std::string& url,
@@ -251,9 +240,6 @@ void Worker::Run() {
251240
Locker locker(isolate_);
252241
Isolate::Scope isolate_scope(isolate_);
253242
SealHandleScope outer_seal(isolate_);
254-
#if HAVE_INSPECTOR
255-
bool inspector_started = false;
256-
#endif
257243

258244
DeleteFnPtr<Environment, FreeEnvironment> env_;
259245
OnScopeLeave cleanup_env([&]() {
@@ -283,10 +269,6 @@ void Worker::Run() {
283269
env_->stop_sub_worker_contexts();
284270
env_->RunCleanup();
285271
RunAtExit(env_.get());
286-
#if HAVE_INSPECTOR
287-
if (inspector_started)
288-
WaitForWorkerInspectorToStop(env_.get());
289-
#endif
290272

291273
// This call needs to be made while the `Environment` is still alive
292274
// because we assume that it is available for async tracking in the
@@ -344,7 +326,6 @@ void Worker::Run() {
344326
env_->InitializeDiagnostics();
345327
#if HAVE_INSPECTOR
346328
env_->InitializeInspector(inspector_parent_handle_.release());
347-
inspector_started = true;
348329
#endif
349330
HandleScope handle_scope(isolate_);
350331
InternalCallbackScope callback_scope(

0 commit comments

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