From e47c13f177f66f3b9e8f4b2027c62b4f5a14f19e Mon Sep 17 00:00:00 2001 From: Bo Date: Tue, 28 Mar 2023 12:03:00 +0800 Subject: [PATCH] fix(napi): check if the tokio runtime exists when registering the module And recreate it if it does not exist. In windows, electron renderer process will crash if: 1. Import some NAPI module that enable `tokio-rt` flag in renderer process. 2. Reload the page. 3. Call a function imported from that NAPI module. Because the tokio runtime will be dropped when reloading the page, and won't create again, but currently we assume that the runtime must exist in tokio-based `within_runtime_if_available`. This will cause some panic like this: ``` thread '' panicked at 'called `Option::unwrap()` on a `None` value', napi-rs\crates\napi\src\tokio_runtime.rs:72:42 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace Error: Renderer process crashed: crashed, exitCode: -529697949 at EventEmitter. (napi-rs\examples\napi\electron.js:33:9) at EventEmitter.emit (node:events:525:35) ``` --- .eslintrc.yml | 2 + .../src/bindgen_runtime/module_register.rs | 2 + crates/napi/src/tokio_runtime.rs | 30 +++++++---- examples/napi/electron-renderer/index.html | 13 +++++ examples/napi/electron-renderer/index.js | 7 +++ examples/napi/electron.js | 51 +++++++++++++++++-- examples/napi/tsconfig.json | 2 +- 7 files changed, 92 insertions(+), 15 deletions(-) create mode 100644 examples/napi/electron-renderer/index.html create mode 100644 examples/napi/electron-renderer/index.js diff --git a/.eslintrc.yml b/.eslintrc.yml index 80ea4098..44351830 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -204,6 +204,8 @@ overrides: parserOptions: project: - ./examples/tsconfig.json + rules: + 'import/no-extraneous-dependencies': 0 - files: - ./bench/**/*.{ts,js} diff --git a/crates/napi/src/bindgen_runtime/module_register.rs b/crates/napi/src/bindgen_runtime/module_register.rs index 5d463bfa..b9d142a7 100644 --- a/crates/napi/src/bindgen_runtime/module_register.rs +++ b/crates/napi/src/bindgen_runtime/module_register.rs @@ -488,6 +488,8 @@ unsafe extern "C" fn napi_register_module_v1( #[cfg(all(windows, feature = "napi4", feature = "tokio_rt"))] { + crate::tokio_runtime::ensure_runtime(); + crate::tokio_runtime::RT_REFERENCE_COUNT.fetch_add(1, Ordering::SeqCst); unsafe { sys::napi_add_env_cleanup_hook( diff --git a/crates/napi/src/tokio_runtime.rs b/crates/napi/src/tokio_runtime.rs index 53d397f4..1b769b5d 100644 --- a/crates/napi/src/tokio_runtime.rs +++ b/crates/napi/src/tokio_runtime.rs @@ -1,11 +1,11 @@ -use std::future::Future; +use std::{future::Future, sync::RwLock}; use once_cell::sync::Lazy; use tokio::runtime::Runtime; use crate::{sys, JsDeferred, JsUnknown, NapiValue, Result}; -pub(crate) static mut RT: Lazy> = Lazy::new(|| { +fn create_runtime() -> Option { #[cfg(not(target_arch = "wasm32"))] { let runtime = tokio::runtime::Runtime::new().expect("Create tokio runtime failed"); @@ -19,20 +19,32 @@ pub(crate) static mut RT: Lazy> = Lazy::new(|| { .build() .ok() } -}); +} + +pub(crate) static RT: Lazy>> = Lazy::new(|| RwLock::new(create_runtime())); #[cfg(windows)] pub(crate) static RT_REFERENCE_COUNT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); +/// Ensure that the Tokio runtime is initialized. +/// In windows the Tokio runtime will be dropped when Node env exits. +/// But in Electron renderer process, the Node env will exits and recreate when the window reloads. +/// So we need to ensure that the Tokio runtime is initialized when the Node env is created. +#[cfg(windows)] +pub(crate) fn ensure_runtime() { + let mut rt = RT.write().unwrap(); + if rt.is_none() { + *rt = create_runtime(); + } +} + #[cfg(windows)] pub(crate) unsafe extern "C" fn drop_runtime(arg: *mut std::ffi::c_void) { use std::sync::atomic::Ordering; if RT_REFERENCE_COUNT.fetch_sub(1, Ordering::SeqCst) == 1 { - if let Some(rt) = Lazy::get_mut(unsafe { &mut RT }) { - rt.take(); - } + RT.write().unwrap().take(); } unsafe { @@ -49,7 +61,7 @@ pub fn spawn(fut: F) -> tokio::task::JoinHandle where F: 'static + Send + Future, { - unsafe { RT.as_ref() }.unwrap().spawn(fut) + RT.read().unwrap().as_ref().unwrap().spawn(fut) } /// Runs a future to completion @@ -59,7 +71,7 @@ pub fn block_on(fut: F) -> F::Output where F: 'static + Send + Future, { - unsafe { RT.as_ref() }.unwrap().block_on(fut) + RT.read().unwrap().as_ref().unwrap().block_on(fut) } // This function's signature must be kept in sync with the one in lib.rs, otherwise napi @@ -69,7 +81,7 @@ where /// then call the provided closure. Otherwise it will just call the provided closure. #[inline] pub fn within_runtime_if_available T, T>(f: F) -> T { - let _rt_guard = unsafe { RT.as_ref() }.unwrap().enter(); + let _rt_guard = RT.read().unwrap().as_ref().unwrap().enter(); f() } diff --git a/examples/napi/electron-renderer/index.html b/examples/napi/electron-renderer/index.html new file mode 100644 index 00000000..0224be71 --- /dev/null +++ b/examples/napi/electron-renderer/index.html @@ -0,0 +1,13 @@ + + + + + + + Electron test + + +
Electron test
+ + + diff --git a/examples/napi/electron-renderer/index.js b/examples/napi/electron-renderer/index.js new file mode 100644 index 00000000..e3cc9965 --- /dev/null +++ b/examples/napi/electron-renderer/index.js @@ -0,0 +1,7 @@ +const { ipcRenderer } = require('electron') + +const { callThreadsafeFunction } = require('../index') + +callThreadsafeFunction(() => {}) + +ipcRenderer.on('ping', () => ipcRenderer.send('pong')) diff --git a/examples/napi/electron.js b/examples/napi/electron.js index 6d3ce1e1..9f9ae8ae 100644 --- a/examples/napi/electron.js +++ b/examples/napi/electron.js @@ -1,6 +1,8 @@ const assert = require('assert') const { readFileSync } = require('fs') +const { app, BrowserWindow, ipcMain } = require('electron') + const { readFileAsync, callThreadsafeFunction, @@ -10,6 +12,42 @@ const { const FILE_CONTENT = readFileSync(__filename, 'utf8') +const createWindowAndReload = async () => { + await app.whenReady() + + const win = new BrowserWindow({ + width: 800, + height: 600, + show: false, + webPreferences: { + nodeIntegration: true, + contextIsolation: false, + }, + }) + + await win.loadFile('./electron-renderer/index.html') + + await new Promise((resolve, reject) => { + win.webContents.on('render-process-gone', (e, detail) => { + reject( + new Error( + `Renderer process crashed: ${detail.reason}, exitCode: ${detail.exitCode}`, + ), + ) + }) + + // reload to check if there is any crash + win.reload() + + // Wait for a while to make sure if a crash happens, the 'resolve' function should be called after the crash + setTimeout(() => { + // make sure the renderer process is still alive + ipcMain.once('pong', () => resolve()) + win.webContents.send('ping') + }, 500) + }) +} + async function main() { const ctrl = new AbortController() const promise = withAbortController(1, 2, ctrl.signal) @@ -45,10 +83,13 @@ async function main() { Array.from({ length: 100 }, (_, i) => i + 1).reduce((a, b) => a + b), ) console.info(createExternalTypedArray()) - process.exit(0) } -main().catch((e) => { - console.error(e) - process.exit(1) -}) +Promise.all([main(), createWindowAndReload()]) + .then(() => { + process.exit(0) + }) + .catch((e) => { + console.error(e) + process.exit(1) + }) diff --git a/examples/napi/tsconfig.json b/examples/napi/tsconfig.json index 73d0267e..6e513db0 100644 --- a/examples/napi/tsconfig.json +++ b/examples/napi/tsconfig.json @@ -7,5 +7,5 @@ "target": "ES2018", "skipLibCheck": false }, - "exclude": ["dist", "electron.js"] + "exclude": ["dist", "electron.js", "electron-renderer"] }