From 00454eb577648eb6aad44844b66662fb01eb1634 Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Tue, 18 Aug 2020 13:55:35 +0800 Subject: [PATCH 1/2] refactor(napi): use get_uv_event_loop instead of uv_default_loop Using the uv_loop_s from napi_env is safer as it's possible that there are multiple Workers, and therefore multiple uv_loop_s. --- napi/src/env.rs | 13 +++++++++++- test_module/__test__/napi4/uv.spec.js | 28 ++++++++++++++++++++++++- test_module/__test__/napi4/uv_worker.js | 16 ++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 test_module/__test__/napi4/uv_worker.js diff --git a/napi/src/env.rs b/napi/src/env.rs index 441f00db..ecb722cc 100644 --- a/napi/src/env.rs +++ b/napi/src/env.rs @@ -533,6 +533,17 @@ impl Env { .map_err(|e| Error::new(Status::InvalidArg, format!("{}", e))) } + #[cfg(napi2)] + #[inline] + pub fn get_uv_event_loop(&self) -> Result<*mut sys::uv_loop_s> { + let mut uv_loop: *mut sys::uv_loop_s = ptr::null_mut(); + Ok(unsafe { + let status = sys::napi_get_uv_event_loop(self.0, &mut uv_loop); + check_status(status)?; + uv_loop + }) + } + #[cfg(all(feature = "libuv", napi4))] #[inline] pub fn execute< @@ -553,7 +564,7 @@ impl Env { check_status(status)?; } - let event_loop = unsafe { sys::uv_default_loop() }; + let event_loop = self.get_uv_event_loop()?; let future_promise = promise::FuturePromise::create(self.0, raw_deferred, Box::from(resolver))?; let future_to_execute = promise::resolve_from_future(future_promise.start()?, deferred); uv::execute(event_loop, Box::pin(future_to_execute))?; diff --git a/test_module/__test__/napi4/uv.spec.js b/test_module/__test__/napi4/uv.spec.js index b719a929..addca3e3 100644 --- a/test_module/__test__/napi4/uv.spec.js +++ b/test_module/__test__/napi4/uv.spec.js @@ -1,10 +1,18 @@ const test = require('ava') -const { join } = require('path') +const { join, resolve } = require('path') const { readFileSync } = require('fs') const bindings = require('../../index.node') const napiVersion = require('../napi-version') +let threadMod + +try { + threadMod = require('worker_threads') +} catch (err) { + // +} + const filepath = join(__dirname, './example.txt') test('should execute future on libuv thread pool', async (t) => { @@ -16,3 +24,21 @@ test('should execute future on libuv thread pool', async (t) => { t.true(Buffer.isBuffer(fileContent)) t.deepEqual(readFileSync(filepath), fileContent) }) + +test('should execute future on libuv thread pool of "Worker"', async (t) => { + // Test in threads if current Node.js supports "worker_threads".` + if (!threadMod || napiVersion < 4) { + return + } + + const { Worker } = threadMod + const script = resolve(__dirname, './uv_worker.js') + const worker = new Worker(script) + const success = await new Promise((resolve) => { + worker.on('message', (success) => { + resolve(success) + }) + }) + + t.is(success, true) +}) diff --git a/test_module/__test__/napi4/uv_worker.js b/test_module/__test__/napi4/uv_worker.js new file mode 100644 index 00000000..dab37c10 --- /dev/null +++ b/test_module/__test__/napi4/uv_worker.js @@ -0,0 +1,16 @@ +const { isMainThread, parentPort } = require('worker_threads') +const { join } = require('path') +const { readFileSync } = require('fs') +const bindings = require('../../index.node') + +const filepath = join(__dirname, './example.txt') + +if (!isMainThread) { + ;(async () => { + const fileContent = await bindings.uvReadFile(filepath) + const success = + Buffer.isBuffer(fileContent) && + readFileSync(filepath).toString('utf8') === fileContent.toString('utf8') + parentPort.postMessage(success) + })() +} From 1329409ca42a792c6e25f250b7d3113a6b206afa Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Tue, 18 Aug 2020 14:11:19 +0800 Subject: [PATCH 2/2] fixup! refactor(napi): use get_uv_event_loop instead of uv_default_loop --- test_module/__test__/napi4/uv.spec.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/test_module/__test__/napi4/uv.spec.js b/test_module/__test__/napi4/uv.spec.js index addca3e3..639e4dc6 100644 --- a/test_module/__test__/napi4/uv.spec.js +++ b/test_module/__test__/napi4/uv.spec.js @@ -25,20 +25,19 @@ test('should execute future on libuv thread pool', async (t) => { t.deepEqual(readFileSync(filepath), fileContent) }) -test('should execute future on libuv thread pool of "Worker"', async (t) => { - // Test in threads if current Node.js supports "worker_threads".` - if (!threadMod || napiVersion < 4) { - return - } +if (threadMod && napiVersion >= 4) { + test('should execute future on libuv thread pool of "Worker"', async (t) => { + // Test in threads if current Node.js supports "worker_threads".` - const { Worker } = threadMod - const script = resolve(__dirname, './uv_worker.js') - const worker = new Worker(script) - const success = await new Promise((resolve) => { - worker.on('message', (success) => { - resolve(success) + const { Worker } = threadMod + const script = resolve(__dirname, './uv_worker.js') + const worker = new Worker(script) + const success = await new Promise((resolve) => { + worker.on('message', (success) => { + resolve(success) + }) }) - }) - t.is(success, true) -}) + t.is(success, true) + }) +}