From 200ec0e2b8df43716890b1e2ec52e9438a311f70 Mon Sep 17 00:00:00 2001 From: jiangheng Date: Tue, 16 Jun 2026 20:50:25 +0800 Subject: [PATCH 1/4] fix: lock onmessage to prevent duplicate message handlers wasm-bindgen registers a message handler during mod.default() initialization. When using addEventListener for task dispatch, both handlers fire for every postMessage, causing the wasm-bindgen handler to fail on wasmworker-format messages. Lock the onmessage setter via Object.defineProperty after init so that addEventListener remains the sole message dispatch path. --- src/webworker/js.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/webworker/js.rs b/src/webworker/js.rs index 704eda2..94d70ba 100644 --- a/src/webworker/js.rs +++ b/src/webworker/js.rs @@ -19,6 +19,16 @@ console.debug('Initializing worker'); } await mod.default({{wasm_bg}}); + + // wasm-bindgen registers a message handler during init. Lock the + // onmessage setter so future assignments can't add duplicate + // listeners alongside our addEventListener handler. + self.onmessage = null; + Object.defineProperty(self, 'onmessage', { + set: function(v) {}, + get: function() { return null; }, + }); + self.postMessage({ success: true }); console.debug('Worker started'); @@ -87,6 +97,14 @@ initHandler = async function(event) { // Remove this listener and add the task handler self.removeEventListener('message', initHandler); + // Lock onmessage to prevent wasm-bindgen's handler from firing + // alongside addEventListener. + self.onmessage = null; + Object.defineProperty(self, 'onmessage', { + set: function(v) {}, + get: function() { return null; }, + }); + // Add the main message handler for tasks self.addEventListener('message', async event => { console.debug('Received worker event'); From e6794f6a507ead677dbadab55cb54336b5f4ce6e Mon Sep 17 00:00:00 2001 From: jiangheng Date: Thu, 16 Jul 2026 06:26:57 +0800 Subject: [PATCH 2/4] test: regression test for conflicting onmessage in workers --- test/Cargo.toml | 2 +- test/src/lib.rs | 5 +++ test/src/onmessage.rs | 89 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 test/src/onmessage.rs diff --git a/test/Cargo.toml b/test/Cargo.toml index accfcca..ab2e0a6 100644 --- a/test/Cargo.toml +++ b/test/Cargo.toml @@ -21,7 +21,7 @@ serde = { version = "1.0", features = ["derive"] } wasm-bindgen = "0.2" js-sys = "0.3" wasm-bindgen-futures = "0.4" -web-sys = { version = "0.3", features = ["MessagePort", "Window"] } +web-sys = { version = "0.3", features = ["DedicatedWorkerGlobalScope", "MessageEvent", "MessagePort", "Window"] } wasmworker = { workspace = true } [features] diff --git a/test/src/lib.rs b/test/src/lib.rs index 554aa65..8213a16 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -1,10 +1,12 @@ use channel::*; use convert::*; +use onmessage::*; use raw::*; use wasm_bindgen::prelude::wasm_bindgen; pub(crate) mod channel; pub(crate) mod convert; +pub(crate) mod onmessage; pub(crate) mod raw; #[macro_export] @@ -46,4 +48,7 @@ pub async fn run_tests() { // Idle timeout test can_use_idle_timeout().await; + + // Conflicting onmessage regression test + can_run_task_with_conflicting_onmessage().await; } diff --git a/test/src/onmessage.rs b/test/src/onmessage.rs new file mode 100644 index 0000000..d670879 --- /dev/null +++ b/test/src/onmessage.rs @@ -0,0 +1,89 @@ +//! Regression test for the `onmessage` lock in `src/webworker/js.rs`. +//! +//! The `#[wasm_bindgen(start)]` function below installs a conflicting +//! `onmessage` handler in every worker (start functions run during +//! `mod.default()`). The worker init code must clear and lock `onmessage` +//! after init so that task dispatch via `addEventListener` remains the +//! only message path. + +use wasm_bindgen::{closure::Closure, prelude::wasm_bindgen, JsCast, JsValue}; +use wasmworker::{webworker, webworker_fn, WebWorker, WebWorkerPool}; +use web_sys::{DedicatedWorkerGlobalScope, MessageEvent}; + +use crate::{js_assert_eq, raw::sort}; + +/// Runs on every module initialization, both on the main page and inside +/// workers. Inside workers, install a hostile `onmessage` handler: if it +/// ever fires, it posts a message to the main thread that cannot be +/// deserialized as a task response, which fails the test run. +#[wasm_bindgen(start)] +fn register_conflicting_onmessage() { + let Ok(scope) = js_sys::global().dyn_into::() else { + // Main page: `self` is a `Window`, nothing to do. + return; + }; + + let post = scope.clone(); + let handler = Closure::::new(move |_: MessageEvent| { + let _ = post.post_message(&JsValue::from_str("conflicting onmessage fired")); + }); + scope.set_onmessage(Some(handler.as_ref().unchecked_ref())); + handler.forget(); +} + +/// Runs inside a worker and reports whether its `onmessage` handler is cleared. +#[webworker_fn] +fn onmessage_is_cleared(_: Box<[u8]>) -> Box<[u8]> { + let cleared = js_sys::global() + .dyn_into::() + .map(|scope| scope.onmessage().is_none()) + .unwrap_or(false); + vec![cleared as u8].into() +} + +/// A conflicting `onmessage` registered by a `#[wasm_bindgen(start)]` function +/// must not break task dispatch. Covers both worker blobs: the regular one +/// (via [`WebWorker`]) and the precompiled-WASM one (via +/// [`WebWorkerPool::with_precompiled_wasm`]). +pub(crate) async fn can_run_task_with_conflicting_onmessage() { + let vec: Box<[u8]> = vec![8, 1, 5, 0, 4].into(); + let sorted: Box<[u8]> = vec![0, 1, 4, 5, 8].into(); + let empty: Box<[u8]> = Vec::new().into(); + let cleared: Box<[u8]> = vec![1].into(); + + // Regular worker blob. + let worker = WebWorker::new(None).await.expect("Couldn't create worker"); + + let res = worker.run_bytes(webworker!(sort), &vec).await; + js_assert_eq!( + res, + sorted, + "Task should complete despite conflicting onmessage" + ); + + let res = worker + .run_bytes(webworker!(onmessage_is_cleared), &empty) + .await; + js_assert_eq!(res, cleared, "onmessage should be cleared in worker"); + + // Precompiled WASM blob. + let pool = WebWorkerPool::with_precompiled_wasm() + .await + .expect("Couldn't create pool with precompiled WASM"); + + let res = pool.run_bytes(webworker!(sort), &vec).await; + js_assert_eq!( + res, + sorted, + "Task should complete despite conflicting onmessage (precompiled)" + ); + + let res = pool + .run_bytes(webworker!(onmessage_is_cleared), &empty) + .await; + js_assert_eq!( + res, + cleared, + "onmessage should be cleared in precompiled worker" + ); +} From e167f09d806d5eed74f5460e0c4eb3dafaccfbbc Mon Sep 17 00:00:00 2001 From: jiangheng Date: Thu, 16 Jul 2026 06:39:04 +0800 Subject: [PATCH 3/4] fix: log warning instead of silently ignoring onmessage assignments --- src/webworker/js.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/webworker/js.rs b/src/webworker/js.rs index 94d70ba..661d149 100644 --- a/src/webworker/js.rs +++ b/src/webworker/js.rs @@ -25,7 +25,7 @@ console.debug('Initializing worker'); // listeners alongside our addEventListener handler. self.onmessage = null; Object.defineProperty(self, 'onmessage', { - set: function(v) {}, + set: function(v) { console.warn('wasmworker: assignment to self.onmessage ignored; message handling is reserved for task dispatch'); }, get: function() { return null; }, }); @@ -101,7 +101,7 @@ initHandler = async function(event) { // alongside addEventListener. self.onmessage = null; Object.defineProperty(self, 'onmessage', { - set: function(v) {}, + set: function(v) { console.warn('wasmworker: assignment to self.onmessage ignored; message handling is reserved for task dispatch'); }, get: function() { return null; }, }); From 00ccbeaad01dcf1094153bc3bd2b2c1959c9db11 Mon Sep 17 00:00:00 2001 From: Pascal Berrang Date: Tue, 21 Jul 2026 11:45:51 +0100 Subject: [PATCH 4/4] Dispatch all worker traffic over a dedicated MessageChannel port The init handshake and task dispatch now run on a MessagePort transferred with the first message to the worker, leaving the worker's global message channel to the embedded module. This replaces the onmessage lock: module handlers (onmessage or addEventListener) can no longer interfere with task dispatch, and module messages posted on the global scope can no longer reach wasmworker's response callback. --- src/webworker/js.rs | 82 ++++++++++++++++++++----------------- src/webworker/worker.rs | 51 +++++++++++++++++------ test/src/onmessage.rs | 90 +++++++++++++++++++++++++---------------- 3 files changed, 138 insertions(+), 85 deletions(-) diff --git a/src/webworker/js.rs b/src/webworker/js.rs index 661d149..f49887e 100644 --- a/src/webworker/js.rs +++ b/src/webworker/js.rs @@ -4,35 +4,46 @@ use wasm_bindgen::prelude::wasm_bindgen; /// The initialization code for the worker, /// which will be loaded as a blob. /// +/// All wasmworker traffic (init handshake and task dispatch) runs over a +/// dedicated `MessageChannel` port, which is transferred with the first +/// message to the worker. This keeps the worker's global message channel +/// free for the embedded module, so message handlers installed by module +/// code (e.g. in a `#[wasm_bindgen(start)]` function) never interfere +/// with task dispatch, and messages posted by the module on the global +/// scope never reach wasmworker's response callback. +/// /// `{{wasm}}` will be replaced later by an actual path. pub(crate) const WORKER_JS: &str = r#" console.debug('Initializing worker'); +// Capture the dedicated task port before any module code can run. +const portPromise = new Promise(resolve => { + const initListener = event => { + if (event.data && event.data.type === 'init_port') { + self.removeEventListener('message', initListener); + resolve(event.ports[0]); + } + }; + self.addEventListener('message', initListener); +}); + (async () => { + const port = await portPromise; + let mod; try { mod = await import('{{wasm}}'); } catch (e) { console.error('Unable to import module {{wasm}}', e); - self.postMessage({ success: false, message: e.toString() }); + port.postMessage({ success: false, message: e.toString() }); return; } await mod.default({{wasm_bg}}); - - // wasm-bindgen registers a message handler during init. Lock the - // onmessage setter so future assignments can't add duplicate - // listeners alongside our addEventListener handler. - self.onmessage = null; - Object.defineProperty(self, 'onmessage', { - set: function(v) { console.warn('wasmworker: assignment to self.onmessage ignored; message handling is reserved for task dispatch'); }, - get: function() { return null; }, - }); - - self.postMessage({ success: true }); + port.postMessage({ success: true }); console.debug('Worker started'); - self.addEventListener('message', async event => { + port.onmessage = async event => { console.debug('Received worker event'); const { id, func_name, is_channel, arg } = event.data; @@ -41,7 +52,7 @@ console.debug('Initializing worker'); const fn = mod[webworker_func_name]; if (!fn) { console.error(`Function '${func_name}' is not exported.`); - self.postMessage({ id: id, response: null }); + port.postMessage({ id: id, response: null }); return; } @@ -49,8 +60,8 @@ console.debug('Initializing worker'); // Send response back to be handled by callback in main thread. console.debug('Send worker result'); - self.postMessage({ id: id, response: worker_result }); - }); + port.postMessage({ id: id, response: worker_result }); + }; })(); "#; @@ -66,47 +77,42 @@ pub(crate) fn main_js() -> JsString { URL.with(Clone::clone) } -/// The initialization code for workers that receive a pre-compiled WASM module +/// The initialization code for workers that receive a pre-compiled WASM module. +/// +/// Like [`WORKER_JS`], all wasmworker traffic runs over a dedicated +/// `MessageChannel` port, which arrives with the `wasm_module` init message. pub(crate) const WORKER_JS_WITH_PRECOMPILED: &str = r#" console.debug('Initializing worker with pre-compiled WASM'); -let wasmModule = null; let mod = null; let initHandler = null; -// Listen for the pre-compiled WASM module +// Listen for the pre-compiled WASM module and the dedicated task port initHandler = async function(event) { const data = event.data; if (data.type === 'wasm_module') { console.debug('Received pre-compiled WASM module'); - wasmModule = data.module; + const port = event.ports[0]; + + // Remove this listener before running module code, so wasmworker + // no longer listens on the global scope at all. + self.removeEventListener('message', initHandler); // Now initialize with the pre-compiled module try { mod = await import('{{wasm}}'); - await mod.default({ module_or_path: wasmModule }); - self.postMessage({ success: true }); + await mod.default({ module_or_path: data.module }); + port.postMessage({ success: true }); console.debug('Worker started with pre-compiled WASM'); } catch (e) { console.error('Unable to initialize with pre-compiled WASM', e); - self.postMessage({ success: false, message: e.toString() }); + port.postMessage({ success: false, message: e.toString() }); return; } - // Remove this listener and add the task handler - self.removeEventListener('message', initHandler); - - // Lock onmessage to prevent wasm-bindgen's handler from firing - // alongside addEventListener. - self.onmessage = null; - Object.defineProperty(self, 'onmessage', { - set: function(v) { console.warn('wasmworker: assignment to self.onmessage ignored; message handling is reserved for task dispatch'); }, - get: function() { return null; }, - }); - // Add the main message handler for tasks - self.addEventListener('message', async event => { + port.onmessage = async event => { console.debug('Received worker event'); const { id, func_name, is_channel, arg } = event.data; @@ -115,7 +121,7 @@ initHandler = async function(event) { const fn = mod[webworker_func_name]; if (!fn) { console.error(`Function '${func_name}' is not exported.`); - self.postMessage({ id: id, response: null }); + port.postMessage({ id: id, response: null }); return; } @@ -123,8 +129,8 @@ initHandler = async function(event) { // Send response back to be handled by callback in main thread. console.debug('Send worker result'); - self.postMessage({ id: id, response: worker_result }); - }); + port.postMessage({ id: id, response: worker_result }); + }; } }; diff --git a/src/webworker/worker.rs b/src/webworker/worker.rs index 0258010..3cdb220 100644 --- a/src/webworker/worker.rs +++ b/src/webworker/worker.rs @@ -51,6 +51,12 @@ type Callback = dyn FnMut(MessageEvent); pub struct WebWorker { /// The underlying web worker. worker: Worker, + /// The dedicated port for all wasmworker traffic (init handshake and + /// task dispatch). Using a `MessageChannel` instead of the worker's + /// global message channel keeps the latter free for the embedded module, + /// so message handlers installed by module code never interfere with + /// task dispatch and vice versa. + port: MessagePort, /// An optional limit on the number of tasks queued at the same time. task_limit: Option, /// The current task id, which is used to reidentify responses. @@ -146,9 +152,17 @@ impl WebWorker { let worker = Worker::new_with_options(&script_url, &worker_options) .map_err(InitError::WebWorkerCreation)?; - // Send pre-compiled WASM module if provided + // Create the dedicated channel for all wasmworker traffic. One port + // stays on the main thread, the other is transferred to the worker + // with the init message. + let channel = MessageChannel::new().map_err(InitError::ChannelCreation)?; + let port = channel.port1(); + let worker_port = channel.port2(); + + // Send the init message with the task port + // (and the pre-compiled WASM module if provided). + let init_msg = js_sys::Object::new(); if let Some(module) = wasm_module { - let init_msg = js_sys::Object::new(); js_sys::Reflect::set( &init_msg, &JsValue::from_str("type"), @@ -157,12 +171,21 @@ impl WebWorker { .expect_throw("Could not set type"); js_sys::Reflect::set(&init_msg, &JsValue::from_str("module"), &module) .expect_throw("Could not set module"); - - worker - .post_message(&init_msg) - .expect_throw("Could not send WASM module to worker"); + } else { + js_sys::Reflect::set( + &init_msg, + &JsValue::from_str("type"), + &JsValue::from_str("init_port"), + ) + .expect_throw("Could not set type"); } + let transfer = Array::new(); + transfer.push(&worker_port); + worker + .post_message_with_transfer(&init_msg, &transfer) + .expect_throw("Could not send init message to worker"); + // Wait until worker is initialized. let (tx, rx) = oneshot::channel(); let handler = Closure::once(move |event: MessageEvent| { @@ -171,7 +194,7 @@ impl WebWorker { .expect_throw("Error deserializing post init data"); let _ = tx.send(post_init); }); - worker.set_onmessage(Some(handler.as_ref().unchecked_ref())); + port.set_onmessage(Some(handler.as_ref().unchecked_ref())); let post_init = rx.await.expect_throw("WebWorker init sender dropped"); // Handle errors in webworker init @@ -187,10 +210,11 @@ impl WebWorker { let last_active = Rc::new(Cell::new(js_sys::Date::now())); let callback_handle = Self::callback(Rc::clone(&tasks), Rc::clone(&last_active)); - worker.set_onmessage(Some(callback_handle.as_ref().unchecked_ref())); + port.set_onmessage(Some(callback_handle.as_ref().unchecked_ref())); Ok(WebWorker { worker, + port, task_limit: task_limit.map(|limit| Semaphore::new(limit)), current_task: AtomicU32::new(0), open_tasks: tasks, @@ -447,15 +471,15 @@ impl WebWorker { let transfer = Array::new(); transfer.push(&port); - self.worker - .post_message_with_transfer( + self.port + .post_message_with_transferable( &serde_wasm_bindgen::to_value(&request) .expect_throw("Could not serialize request"), &transfer, ) .expect_throw("WebWorker gone"); } else { - self.worker + self.port .post_message( &serde_wasm_bindgen::to_value(&request) .expect_throw("Could not serialize request"), @@ -497,8 +521,8 @@ impl WebWorker { let transfer = Array::new(); transfer.push(&port); - self.worker - .post_message_with_transfer( + self.port + .post_message_with_transferable( &serde_wasm_bindgen::to_value(&request).expect_throw("Could not serialize request"), &transfer, ) @@ -534,6 +558,7 @@ impl WebWorker { impl Drop for WebWorker { fn drop(&mut self) { + self.port.close(); self.worker.terminate(); } } diff --git a/test/src/onmessage.rs b/test/src/onmessage.rs index d670879..f6ec5cf 100644 --- a/test/src/onmessage.rs +++ b/test/src/onmessage.rs @@ -1,55 +1,69 @@ -//! Regression test for the `onmessage` lock in `src/webworker/js.rs`. +//! Regression test for conflicting message handlers in workers. //! -//! The `#[wasm_bindgen(start)]` function below installs a conflicting -//! `onmessage` handler in every worker (start functions run during -//! `mod.default()`). The worker init code must clear and lock `onmessage` -//! after init so that task dispatch via `addEventListener` remains the -//! only message path. +//! The `#[wasm_bindgen(start)]` function below installs hostile message +//! handlers in every worker (start functions run during `mod.default()`), +//! both via the `onmessage` property and via `addEventListener`. Since all +//! wasmworker traffic runs over a dedicated `MessageChannel` port, these +//! handlers must never fire, and messages the module posts on the global +//! scope must never reach wasmworker's response callback. -use wasm_bindgen::{closure::Closure, prelude::wasm_bindgen, JsCast, JsValue}; +use wasm_bindgen::{closure::Closure, prelude::wasm_bindgen, JsCast, JsValue, UnwrapThrowExt}; use wasmworker::{webworker, webworker_fn, WebWorker, WebWorkerPool}; use web_sys::{DedicatedWorkerGlobalScope, MessageEvent}; use crate::{js_assert_eq, raw::sort}; /// Runs on every module initialization, both on the main page and inside -/// workers. Inside workers, install a hostile `onmessage` handler: if it -/// ever fires, it posts a message to the main thread that cannot be -/// deserialized as a task response, which fails the test run. +/// workers. Inside workers, install hostile message handlers on the global +/// scope: if one of them ever fires, it posts a message to the main thread +/// that cannot be deserialized as a task response. #[wasm_bindgen(start)] -fn register_conflicting_onmessage() { +fn register_conflicting_handlers() { let Ok(scope) = js_sys::global().dyn_into::() else { // Main page: `self` is a `Window`, nothing to do. return; }; + // Via the `onmessage` property, as e.g. wasm-bindgen glue may set it. let post = scope.clone(); let handler = Closure::::new(move |_: MessageEvent| { let _ = post.post_message(&JsValue::from_str("conflicting onmessage fired")); }); scope.set_onmessage(Some(handler.as_ref().unchecked_ref())); handler.forget(); + + // Via `addEventListener`. + let post = scope.clone(); + let listener = Closure::::new(move |_: MessageEvent| { + let _ = post.post_message(&JsValue::from_str("conflicting listener fired")); + }); + scope + .add_event_listener_with_callback("message", listener.as_ref().unchecked_ref()) + .expect_throw("Could not add hostile listener"); + listener.forget(); } -/// Runs inside a worker and reports whether its `onmessage` handler is cleared. +/// Runs inside a worker: posts garbage on the global scope (as a module might +/// do for its own purposes) and returns its argument sorted. #[webworker_fn] -fn onmessage_is_cleared(_: Box<[u8]>) -> Box<[u8]> { - let cleared = js_sys::global() +fn post_garbage(mut v: Box<[u8]>) -> Box<[u8]> { + js_sys::global() .dyn_into::() - .map(|scope| scope.onmessage().is_none()) - .unwrap_or(false); - vec![cleared as u8].into() + .expect_throw("Not in a worker") + .post_message(&JsValue::from_str("garbage from module code")) + .expect_throw("Could not post garbage"); + v.sort(); + v } -/// A conflicting `onmessage` registered by a `#[wasm_bindgen(start)]` function -/// must not break task dispatch. Covers both worker blobs: the regular one -/// (via [`WebWorker`]) and the precompiled-WASM one (via -/// [`WebWorkerPool::with_precompiled_wasm`]). +/// Hostile message handlers registered by a `#[wasm_bindgen(start)]` function +/// must not break task dispatch, and module messages posted on the global +/// scope must not reach wasmworker's response callback. Covers both worker +/// blobs: the regular one (via [`WebWorker`]) and the precompiled-WASM one +/// (via [`WebWorkerPool::with_precompiled_wasm`]). pub(crate) async fn can_run_task_with_conflicting_onmessage() { let vec: Box<[u8]> = vec![8, 1, 5, 0, 4].into(); let sorted: Box<[u8]> = vec![0, 1, 4, 5, 8].into(); - let empty: Box<[u8]> = Vec::new().into(); - let cleared: Box<[u8]> = vec![1].into(); // Regular worker blob. let worker = WebWorker::new(None).await.expect("Couldn't create worker"); @@ -58,13 +72,16 @@ pub(crate) async fn can_run_task_with_conflicting_onmessage() { js_assert_eq!( res, sorted, - "Task should complete despite conflicting onmessage" + "Task should complete despite conflicting handlers" ); - let res = worker - .run_bytes(webworker!(onmessage_is_cleared), &empty) - .await; - js_assert_eq!(res, cleared, "onmessage should be cleared in worker"); + // Worker -> main direction: garbage on the global scope must not disturb + // the response path. + let res = worker.run_bytes(webworker!(post_garbage), &vec).await; + js_assert_eq!(res, sorted, "Task posting garbage should complete"); + + let res = worker.run_bytes(webworker!(sort), &vec).await; + js_assert_eq!(res, sorted, "Tasks should still work after posted garbage"); // Precompiled WASM blob. let pool = WebWorkerPool::with_precompiled_wasm() @@ -75,15 +92,20 @@ pub(crate) async fn can_run_task_with_conflicting_onmessage() { js_assert_eq!( res, sorted, - "Task should complete despite conflicting onmessage (precompiled)" + "Task should complete despite conflicting handlers (precompiled)" + ); + + let res = pool.run_bytes(webworker!(post_garbage), &vec).await; + js_assert_eq!( + res, + sorted, + "Task posting garbage should complete (precompiled)" ); - let res = pool - .run_bytes(webworker!(onmessage_is_cleared), &empty) - .await; + let res = pool.run_bytes(webworker!(sort), &vec).await; js_assert_eq!( res, - cleared, - "onmessage should be cleared in precompiled worker" + sorted, + "Tasks should still work after posted garbage (precompiled)" ); }