fix: potential `async_hooks` crash in `NotifyWindowRestore` on Windows (#41146)

* fix: potential async_hooks crash in NotifyWindowRestore on Windows

Co-authored-by: Shelley Vohr <[email protected]>

* fix: don't use CallbackScope for Error objects

Co-authored-by: Shelley Vohr <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>

shell/browser/api/electron_api_auto_updater.cc

@@ -32,13 +32,26 @@ void AutoUpdater::OnError(const std::string& message) {
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   v8::HandleScope handle_scope(isolate);
   v8::Local<v8::Object> wrapper;
+
+  // We do not use gin::EmitEvent here because we do not want to
+  // put this in its own CallbackScope and delegate to Node.js'
+  // specialized handling for Error objects.
   if (GetWrapper(isolate).ToLocal(&wrapper)) {
     auto error = v8::Exception::Error(gin::StringToV8(isolate, message));
-    gin_helper::EmitEvent(
-        isolate, wrapper, "error",
-        error->ToObject(isolate->GetCurrentContext()).ToLocalChecked(),
-        // Message is also emitted to keep compatibility with old code.
-        message);
+    std::vector<v8::Local<v8::Value>> args = {
+        gin::StringToV8(isolate, "error"),
+        gin::ConvertToV8(
+            isolate,
+            error->ToObject(isolate->GetCurrentContext()).ToLocalChecked()),
+        gin::StringToV8(isolate, message),
+    };
+
+    gin_helper::MicrotasksScope microtasks_scope(
+        isolate, wrapper->GetCreationContextChecked()->GetMicrotaskQueue(),
+        true);
+
+    node::MakeCallback(isolate, wrapper, "emit", args.size(), args.data(),
+                       {0, 0});
   }
 }
 

shell/common/gin_helper/event_emitter_caller.cc

@@ -13,6 +13,14 @@ v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
                                         v8::Local<v8::Object> obj,
                                         const char* method,
                                         ValueVector* args) {
+  // Only set up the node::CallbackScope if there's a node environment.
+  std::unique_ptr<node::CallbackScope> callback_scope;
+  if (node::Environment::GetCurrent(isolate)) {
+    v8::HandleScope handle_scope(isolate);
+    callback_scope = std::make_unique<node::CallbackScope>(
+        isolate, v8::Object::New(isolate), node::async_context{0, 0});
+  }
+
   // Perform microtask checkpoint after running JavaScript.
   gin_helper::MicrotasksScope microtasks_scope(
       isolate, obj->GetCreationContextChecked()->GetMicrotaskQueue(), true);

shell/renderer/electron_api_service_impl.cc

@@ -61,12 +61,10 @@ void InvokeIpcCallback(v8::Local<v8::Context> context,
 
   // Only set up the node::CallbackScope if there's a node environment.
   // Sandboxed renderers don't have a node environment.
-  node::Environment* env = node::Environment::GetCurrent(context);
   std::unique_ptr<node::CallbackScope> callback_scope;
-  if (env) {
-    node::async_context async_context = {};
-    callback_scope = std::make_unique<node::CallbackScope>(isolate, ipcNative,
-                                                           async_context);
+  if (node::Environment::GetCurrent(context)) {
+    callback_scope = std::make_unique<node::CallbackScope>(
+        isolate, ipcNative, node::async_context{0, 0});
   }
 
   auto callback_key = gin::ConvertToV8(isolate, callback_name)

spec/api-browser-window-spec.ts

@@ -14,6 +14,7 @@ import { closeWindow, closeAllWindows } from './lib/window-helpers';
 import { areColorsSimilar, captureScreen, HexColors, getPixelColor } from './lib/screen-helpers';
 import { once } from 'node:events';
 import { setTimeout } from 'node:timers/promises';
+import { setTimeout as syncSetTimeout } from 'node:timers';
 
 const fixtures = path.resolve(__dirname, 'fixtures');
 const mainFixtures = path.resolve(__dirname, 'fixtures');
@@ -1809,6 +1810,36 @@ describe('BrowserWindow module', () => {
           expect(w.isFullScreen()).to.equal(true);
         });
 
+        it('does not crash if maximized, minimized, then restored to maximized state', (done) => {
+          w.destroy();
+          w = new BrowserWindow({ show: false });
+
+          w.show();
+
+          let count = 0;
+
+          w.on('maximize', () => {
+            if (count === 0) syncSetTimeout(() => { w.minimize(); });
+            count++;
+          });
+
+          w.on('minimize', () => {
+            if (count === 1) syncSetTimeout(() => { w.restore(); });
+            count++;
+          });
+
+          w.on('restore', () => {
+            try {
+              throw new Error('hey!');
+            } catch (e: any) {
+              expect(e.message).to.equal('hey!');
+              done();
+            }
+          });
+
+          w.maximize();
+        });
+
         it('checks normal bounds for maximized transparent window', async () => {
           w.destroy();
           w = new BrowserWindow({