Browse Source

fix: crash with --inspect-brk when running under ELECTRON_RUN_AS_NODE (#20098)

* fix: ensure that the node env is not bootstrapped before running inspector

* fix: ensure we wait for the inspect to disconnect

This re-orders our node clean up so that we free the environment before
the task runner is cleaned up as node uses the task runner during clean
up.  It also calls WaitForDisconnect() to ensure that inspector agents
are notified that the context is going down.

* chore: update spec to catch SIGABRT
Robo 5 years ago
parent
commit
4662ab9b63
3 changed files with 31 additions and 15 deletions
  1. 19 13
      atom/app/node_main.cc
  2. 3 1
      atom/browser/node_debugger.cc
  3. 9 1
      spec/node-spec.js

+ 19 - 13
atom/app/node_main.cc

@@ -47,15 +47,6 @@ int NodeMain(int argc, char* argv[]) {
     feature_list->InitializeFromCommandLine("", "");
     base::FeatureList::SetInstance(std::move(feature_list));
 
-    gin::V8Initializer::LoadV8Snapshot(
-        gin::V8Initializer::V8SnapshotFileType::kWithAdditionalContext);
-    gin::V8Initializer::LoadV8Natives();
-
-    // V8 requires a task scheduler apparently
-    base::ThreadPoolInstance::CreateAndStartWithDefaultParams("Electron");
-
-    // Initialize gin::IsolateHolder.
-    JavascriptEnvironment gin_env(loop);
 #if defined(_WIN64)
     crash_reporter::CrashReporterWin::SetUnhandledExceptionFilter();
 #endif
@@ -67,14 +58,26 @@ int NodeMain(int argc, char* argv[]) {
     const char** exec_argv;
     node::Init(&argc, const_cast<const char**>(argv), &exec_argc, &exec_argv);
 
+    gin::V8Initializer::LoadV8Snapshot(
+        gin::V8Initializer::V8SnapshotFileType::kWithAdditionalContext);
+    gin::V8Initializer::LoadV8Natives();
+
+    // V8 requires a task scheduler apparently
+    base::ThreadPoolInstance::CreateAndStartWithDefaultParams("Electron");
+
+    // Initialize gin::IsolateHolder.
+    JavascriptEnvironment gin_env(loop);
+
     node::Environment* env = node::CreateEnvironment(
         node::CreateIsolateData(gin_env.isolate(), loop, gin_env.platform()),
-        gin_env.context(), argc, argv, exec_argc, exec_argv);
+        gin_env.context(), argc, argv, exec_argc, exec_argv, false);
 
     // Enable support for v8 inspector.
     NodeDebugger node_debugger(env);
     node_debugger.Start();
 
+    node::BootstrapEnvironment(env);
+
     mate::Dictionary process(gin_env.isolate(), env->process_object());
 #if defined(OS_WIN)
     process.SetMethod("log", &ElectronBindings::Log);
@@ -110,12 +113,15 @@ int NodeMain(int argc, char* argv[]) {
 
     node_debugger.Stop();
     exit_code = node::EmitExit(env);
+    env->set_can_call_into_js(false);
     node::RunAtExit(env);
-    gin_env.platform()->DrainTasks(env->isolate());
-    gin_env.platform()->CancelPendingDelayedTasks(env->isolate());
-    gin_env.platform()->UnregisterIsolate(env->isolate());
 
+    v8::Isolate* isolate = env->isolate();
     node::FreeEnvironment(env);
+
+    gin_env.platform()->DrainTasks(isolate);
+    gin_env.platform()->CancelPendingDelayedTasks(isolate);
+    gin_env.platform()->UnregisterIsolate(isolate);
   }
 
   // According to "src/gin/shell/gin_main.cc":

+ 3 - 1
atom/browser/node_debugger.cc

@@ -60,8 +60,10 @@ void NodeDebugger::Start() {
 
 void NodeDebugger::Stop() {
   auto* inspector = env_->inspector_agent();
-  if (inspector && inspector->IsListening())
+  if (inspector && inspector->IsListening()) {
+    inspector->WaitForDisconnect();
     inspector->Stop();
+  }
 }
 
 }  // namespace atom

+ 9 - 1
spec/node-spec.js

@@ -311,13 +311,20 @@ describe('node feature', () => {
         output += data
         if (output.trim().startsWith('Debugger listening on ws://')) {
           cleanup()
-          done()
+          child.kill('SIGTERM')
         }
       }
       function outDataHandler (data) {
         cleanup()
         done(new Error(`Unexpected output: ${data.toString()}`))
       }
+      child.on('close', (code, signal) => {
+        if (signal !== 'SIGTERM') {
+          done(new Error(`Unexpected termination with code: ${code} and signal: ${signal}`))
+        } else {
+          done()
+        }
+      })
       child.stderr.on('data', errorDataListener)
       child.stdout.on('data', outDataHandler)
     })
@@ -390,6 +397,7 @@ describe('node feature', () => {
             const connection = new WebSocket(socketMatch[0])
             connection.onopen = () => {
               child.send('plz-quit')
+              connection.close()
               done()
             }
           }