From 665ac6f9c830edbf85596369b49844e1b910c59a Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 22 Mar 2021 20:11:03 +0000 Subject: [PATCH] fix: libuv hang on Windows (#28175) --- shell/common/node_bindings.cc | 4 +-- shell/common/node_bindings.h | 4 +-- spec-main/fixtures/apps/libuv-hang/index.html | 13 +++++++ spec-main/fixtures/apps/libuv-hang/main.js | 36 +++++++++++++++++++ spec-main/fixtures/apps/libuv-hang/preload.js | 16 +++++++++ .../fixtures/apps/libuv-hang/renderer.js | 8 +++++ spec-main/node-spec.ts | 11 ++++++ 7 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 spec-main/fixtures/apps/libuv-hang/index.html create mode 100644 spec-main/fixtures/apps/libuv-hang/main.js create mode 100644 spec-main/fixtures/apps/libuv-hang/preload.js create mode 100644 spec-main/fixtures/apps/libuv-hang/renderer.js diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 8f36ce62de5..91bf1125548 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -534,15 +534,13 @@ void NodeBindings::LoadEnvironment(node::Environment* env) { void NodeBindings::PrepareMessageLoop() { #if !defined(OS_WIN) int handle = uv_backend_fd(uv_loop_); -#else - HANDLE handle = uv_loop_->iocp; -#endif // If the backend fd hasn't changed, don't proceed. if (handle == handle_) return; handle_ = handle; +#endif // Add dummy handle for libuv, otherwise libuv would quit when there is // nothing to do. diff --git a/shell/common/node_bindings.h b/shell/common/node_bindings.h index efc8b40364e..3af029e387e 100644 --- a/shell/common/node_bindings.h +++ b/shell/common/node_bindings.h @@ -159,9 +159,7 @@ class NodeBindings { // Isolate data used in creating the environment node::IsolateData* isolate_data_ = nullptr; -#if defined(OS_WIN) - HANDLE handle_; -#else +#if !defined(OS_WIN) int handle_ = -1; #endif diff --git a/spec-main/fixtures/apps/libuv-hang/index.html b/spec-main/fixtures/apps/libuv-hang/index.html new file mode 100644 index 00000000000..a3534d419a6 --- /dev/null +++ b/spec-main/fixtures/apps/libuv-hang/index.html @@ -0,0 +1,13 @@ + + + + + + + Hello World! + + +

Hello World!

+ + + diff --git a/spec-main/fixtures/apps/libuv-hang/main.js b/spec-main/fixtures/apps/libuv-hang/main.js new file mode 100644 index 00000000000..4ca4ef15d90 --- /dev/null +++ b/spec-main/fixtures/apps/libuv-hang/main.js @@ -0,0 +1,36 @@ +const { app, BrowserWindow, ipcMain } = require('electron'); +const path = require('path'); + +async function createWindow () { + const mainWindow = new BrowserWindow({ + show: false, + webPreferences: { + preload: path.join(__dirname, 'preload.js') + } + }); + + await mainWindow.loadFile('index.html'); +} + +app.whenReady().then(() => { + createWindow(); + app.on('activate', function () { + if (BrowserWindow.getAllWindows().length === 0) { + createWindow(); + } + }); +}); + +let count = 0; +ipcMain.handle('reload-successful', () => { + if (count === 2) { + app.quit(); + } else { + count++; + return count; + } +}); + +app.on('window-all-closed', function () { + if (process.platform !== 'darwin') app.quit(); +}); diff --git a/spec-main/fixtures/apps/libuv-hang/preload.js b/spec-main/fixtures/apps/libuv-hang/preload.js new file mode 100644 index 00000000000..a5840f557f5 --- /dev/null +++ b/spec-main/fixtures/apps/libuv-hang/preload.js @@ -0,0 +1,16 @@ +const { contextBridge, ipcRenderer } = require('electron'); + +contextBridge.exposeInMainWorld('api', { + ipcRenderer, + run: async () => { + const { promises: fs } = require('fs'); + for (let i = 0; i < 10; i++) { + const list = await fs.readdir('.', { withFileTypes: true }); + for (const file of list) { + if (file.isFile()) { + await fs.readFile(file.name, 'utf-8'); + } + } + } + } +}); diff --git a/spec-main/fixtures/apps/libuv-hang/renderer.js b/spec-main/fixtures/apps/libuv-hang/renderer.js new file mode 100644 index 00000000000..5f0a2b58b51 --- /dev/null +++ b/spec-main/fixtures/apps/libuv-hang/renderer.js @@ -0,0 +1,8 @@ +const count = localStorage.getItem('count'); + +const { run, ipcRenderer } = window.api; + +run().then(async () => { + const count = await ipcRenderer.invoke('reload-successful'); + if (count < 3) location.reload(); +}).catch(console.log); diff --git a/spec-main/node-spec.ts b/spec-main/node-spec.ts index def9ad1e465..45396fdd318 100644 --- a/spec-main/node-spec.ts +++ b/spec-main/node-spec.ts @@ -7,6 +7,7 @@ import { ifdescribe, ifit } from './spec-helpers'; import { webContents, WebContents } from 'electron/main'; const features = process._linkedBinding('electron_common_features'); +const mainFixturesPath = path.resolve(__dirname, 'fixtures'); describe('node feature', () => { const fixtures = path.join(__dirname, '..', 'spec', 'fixtures'); @@ -22,6 +23,16 @@ describe('node feature', () => { }); }); + it('does not hang when using the fs module in the renderer process', async () => { + const appPath = path.join(mainFixturesPath, 'apps', 'libuv-hang', 'main.js'); + const appProcess = childProcess.spawn(process.execPath, [appPath], { + cwd: path.join(mainFixturesPath, 'apps', 'libuv-hang'), + stdio: 'inherit' + }); + const [code] = await emittedOnce(appProcess, 'close'); + expect(code).to.equal(0); + }); + describe('contexts', () => { describe('setTimeout called under Chromium event loop in browser process', () => { it('Can be scheduled in time', (done) => {