Skip to content

Commit

Permalink
fix: potential async_hooks crash in NotifyWindowRestore on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Dec 11, 2023
1 parent 2eb13d3 commit 7a41ee7
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
9 changes: 9 additions & 0 deletions shell/common/gin_helper/event_emitter_caller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
const char* method,
ValueVector* args) {
v8::HandleScope handle_scope(isolate);
std::unique_ptr<node::CallbackScope> callback_scope;

// Only set up the node::CallbackScope if there's a node environment.
if (node::Environment::GetCurrent(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);
Expand Down
8 changes: 3 additions & 5 deletions shell/renderer/electron_api_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 31 additions & 0 deletions spec/api-browser-window-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Check failure on line 16 in spec/api-browser-window-spec.ts

View check run for this annotation

trop / Backportable? - 26-x-y

spec/api-browser-window-spec.ts#L15-L16

Patch Conflict
Raw output
++<<<<<<< HEAD
 +import { once } from 'events';
 +import { setTimeout } from 'timers/promises';
++=======
+ import { once } from 'node:events';
+ import { setTimeout } from 'node:timers/promises';
+ import { setTimeout as syncSetTimeout } from 'node:timers';
++>>>>>>> fix: potential async_hooks crash in NotifyWindowRestore on Windows
import { setTimeout as syncSetTimeout } from 'node:timers';

const fixtures = path.resolve(__dirname, 'fixtures');
const mainFixtures = path.resolve(__dirname, 'fixtures');
Expand Down Expand Up @@ -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({
Expand Down

0 comments on commit 7a41ee7

Please sign in to comment.