mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

fix: crash when destroying node env with pending promises (#33280)

Browse source 
* fix: crash when destroying node env with pending promises

* chore: add spec
This commit is contained in:
Robo 2022-03-17 02:54:45 +09:00 • committed by GitHub
parent 652680e801
commit b2c5623a13
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 64 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

19
shell/renderer/electron_renderer_client.cc
View file

@ -151,17 +151,22 @@ void ElectronRendererClient::WillReleaseScriptContext(
if (env == node_bindings_->uv_env())
node_bindings_->set_uv_env(nullptr);
// Destroy the node environment. We only do this if node support has been
// enabled for sub-frames to avoid a change-of-behavior / introduce crashes
// for existing users.
// We also do this if we have disable electron site instance overrides to
// avoid memory leaks
auto prefs = render_frame->GetBlinkPreferences();
gin_helper::MicrotasksScope microtasks_scope(env->isolate());
// Destroying the node environment will also run the uv loop,
// Node.js expects `kExplicit` microtasks policy and will run microtasks
// checkpoints after every call into JavaScript. Since we use a different
// policy in the renderer - switch to `kExplicit` and then drop back to the
// previous policy value.
v8::Isolate* isolate = context->GetIsolate();
auto old_policy = isolate->GetMicrotasksPolicy();
DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(isolate), 0);
isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
node::FreeEnvironment(env);
if (env == node_bindings_->uv_env())
node::FreeIsolateData(node_bindings_->isolate_data());
isolate->SetMicrotasksPolicy(old_policy);
// ElectronBindings is tracking node environments.
electron_bindings_->EnvironmentDestroyed(env);
}

9
shell/renderer/web_worker_observer.cc
View file

@ -37,8 +37,13 @@ WebWorkerObserver::WebWorkerObserver()
WebWorkerObserver::~WebWorkerObserver() {
lazy_tls.Pointer()->Set(nullptr);
gin_helper::MicrotasksScope microtasks_scope(
node_bindings_->uv_env()->isolate());
// Destroying the node environment will also run the uv loop,
// Node.js expects `kExplicit` microtasks policy and will run microtasks
// checkpoints after every call into JavaScript. Since we use a different
// policy in the renderer - switch to `kExplicit`
v8::Isolate* isolate = node_bindings_->uv_env()->isolate();
DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(isolate), 0);
isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
node::FreeEnvironment(node_bindings_->uv_env());
node::FreeIsolateData(node_bindings_->isolate_data());
}

17
spec-main/fixtures/crash-cases/fs-promises-renderer-crash/index.html Normal file
View file

@ -0,0 +1,17 @@
<html>
<body>
<script>
const fs = require('fs');
const { ipcRenderer } = require('electron');
async function readFile(path) {
await fs.promises.readFile(path);
}
ipcRenderer.on('reload', (_, path) => {
readFile(path);
window.location.reload();
});
</script>
</body>
</html>

28
spec-main/fixtures/crash-cases/fs-promises-renderer-crash/index.js Normal file
View file

@ -0,0 +1,28 @@
const { app, BrowserWindow, ipcMain } = require('electron');
const path = require('path');
app.whenReady().then(() => {
let reloadCount = 0;
const win = new BrowserWindow({
show: false,
webPreferences: {
nodeIntegration: true,
contextIsolation: false
}
});
win.loadFile('index.html');
win.webContents.on('render-process-gone', () => {
process.exit(1);
});
win.webContents.on('did-finish-load', () => {
if (reloadCount > 2) {
setImmediate(() => app.quit());
} else {
reloadCount += 1;
win.webContents.send('reload', path.join(__dirname, '..', '..', 'cat.pdf'));
}
});
});