From 07318e514613a276649197c83451420f8bdded50 Mon Sep 17 00:00:00 2001
From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com>
Date: Wed, 11 Nov 2020 10:23:30 -0500
Subject: [PATCH] fix: renderer crash on setImmediate (#26424)

* fix: renderer crash on setImmediate

* spec: add a crash case

* fix: ensure env exists

* Update spec-main/fixtures/crash-cases/setimmediate-renderer-crash/index.js

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: Jeremy Rose <jeremya@chromium.org>
---
 shell/common/node_bindings.cc                 | 23 +++++++++++++++++++
 .../setimmediate-renderer-crash/index.js      | 22 ++++++++++++++++++
 .../setimmediate-renderer-crash/preload.js    |  3 +++
 3 files changed, 48 insertions(+)
 create mode 100644 spec-main/fixtures/crash-cases/setimmediate-renderer-crash/index.js
 create mode 100644 spec-main/fixtures/crash-cases/setimmediate-renderer-crash/preload.js

diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc
index e6399df15f44c..2044186d3610e 100644
--- a/shell/common/node_bindings.cc
+++ b/shell/common/node_bindings.cc
@@ -168,6 +168,23 @@ bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
       context, v8::String::Empty(isolate));
 }
 
+void ErrorMessageListener(v8::Local<v8::Message> message,
+                          v8::Local<v8::Value> data) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  node::Environment* env = node::Environment::GetCurrent(isolate);
+
+  // TODO(codebytere): properly emit the after() hooks now
+  // that the exception has been handled.
+  // See node/lib/internal/process/execution.js#L176-L180
+
+  // Ensure that the async id stack is properly cleared so the async
+  // hook stack does not become corrupted.
+
+  if (env) {
+    env->async_hooks()->clear_async_id_stack();
+  }
+}
+
 // Initialize Node.js cli options to pass to Node.js
 // See https://nodejs.org/api/cli.html#cli_options
 void SetNodeCliFlags() {
@@ -474,6 +491,12 @@ node::Environment* NodeBindings::CreateEnvironment(
     // Blink's.
     is.flags &= ~node::IsolateSettingsFlags::MESSAGE_LISTENER_WITH_ERROR_LEVEL;
 
+    // Isolate message listeners are additive (you can add multiple), so instead
+    // we add an extra one here to ensure that the async hook stack is properly
+    // cleared when errors are thrown.
+    context->GetIsolate()->AddMessageListenerWithErrorLevel(
+        ErrorMessageListener, v8::Isolate::kMessageError);
+
     // We do not want to use the promise rejection callback that Node.js uses,
     // because it does not send PromiseRejectionEvents to the global script
     // context. We need to use the one Blink already provides.
diff --git a/spec-main/fixtures/crash-cases/setimmediate-renderer-crash/index.js b/spec-main/fixtures/crash-cases/setimmediate-renderer-crash/index.js
new file mode 100644
index 0000000000000..7960616214020
--- /dev/null
+++ b/spec-main/fixtures/crash-cases/setimmediate-renderer-crash/index.js
@@ -0,0 +1,22 @@
+const { app, BrowserWindow } = require('electron');
+const path = require('path');
+
+app.whenReady().then(() => {
+  const win = new BrowserWindow({
+    show: false,
+    webPreferences: {
+      nodeIntegration: true,
+      preload: path.resolve(__dirname, 'preload.js')
+    }
+  });
+
+  win.loadURL('about:blank');
+
+  win.webContents.on('render-process-gone', () => {
+    process.exit(1);
+  });
+
+  win.webContents.on('did-finish-load', () => {
+    setTimeout(() => app.quit());
+  });
+});
diff --git a/spec-main/fixtures/crash-cases/setimmediate-renderer-crash/preload.js b/spec-main/fixtures/crash-cases/setimmediate-renderer-crash/preload.js
new file mode 100644
index 0000000000000..8e8afb06af723
--- /dev/null
+++ b/spec-main/fixtures/crash-cases/setimmediate-renderer-crash/preload.js
@@ -0,0 +1,3 @@
+setImmediate(() => {
+  throw new Error('oh no');
+});