fix: libuv hang on Windows (#28336)

* fix: libuv hang on Windows * test: add a hang test * fix: contextIsolation issue Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
electron · Apr 28, 2021 · bb031bf · bb031bf
1 parent f553395
commit bb031bf
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 6 deletions.
diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc
@@ -539,15 +539,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
@@ -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
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="UTF-8">
+    <meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self'">
+    <meta http-equiv="X-Content-Security-Policy" content="default-src 'self'; script-src 'self'">
+    <title>Hello World!</title>
+  </head>
+  <body>
+    <h1>Hello World!</h1>
+    <script src="./renderer.js"></script>
+  </body>
+</html>
diff --git a/spec-main/fixtures/apps/libuv-hang/main.js b/spec-main/fixtures/apps/libuv-hang/main.js
@@ -0,0 +1,37 @@
+const { app, BrowserWindow, ipcMain } = require('electron');
+const path = require('path');
+
+async function createWindow () {
+  const mainWindow = new BrowserWindow({
+    show: false,
+    webPreferences: {
+      contextIsolation: true,
+      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
@@ -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
@@ -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
@@ -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) => {