Skip to content

Commit 92c0e02

Browse files
triniwizclaude
andcommitted
feat: deliver worker callbacks on the worker's thread, unbox Object values
- WinRT delegates and .NET callbacks created in a worker run on that worker's thread when invoked from another thread (the worker loop now waits on commands, messages and timers). - A callback whose worker has terminated is refused instead of running on a disposed isolate. - Messages a worker posts on its own reach the main thread without it polling. - The classic engine unboxes IPropertyValue results (Tag, PropertySet values) to JS primitives. - test-app: worker and boxing specs; hooks of fully skipped suites don't run. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a6666c5 commit 92c0e02

13 files changed

Lines changed: 586 additions & 88 deletions

File tree

‎runtime/src/global_fns.rs‎

Lines changed: 91 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,46 @@ pub(crate) fn handle_worker_post_message(
10801080
}
10811081
}
10821082

1083+
/// Runs on a worker's creating thread when the worker has queued messages: hands them to that
1084+
/// `Worker` object (`__nsWorkerDeliver`, installed by the Worker shim).
1085+
pub(crate) fn deliver_worker_events(worker_id: u64) {
1086+
let isolate_ptr = crate::DELEGATE_ISOLATE_PTR.with(|c| c.get());
1087+
if isolate_ptr.is_null() {
1088+
return;
1089+
}
1090+
let isolate: &mut v8::Isolate = unsafe { &mut *isolate_ptr };
1091+
v8::scope!(scope, isolate);
1092+
let Some(ctx_global) = scope.get_slot::<v8::Global<v8::Context>>().cloned() else {
1093+
return;
1094+
};
1095+
let context = v8::Local::new(scope, &ctx_global);
1096+
let scope = &mut v8::ContextScope::new(scope, context);
1097+
v8::tc_scope!(tc, scope);
1098+
1099+
let global = context.global(tc);
1100+
let Some(key) = v8::String::new(tc, "__nsWorkerDeliver") else {
1101+
return;
1102+
};
1103+
let Some(deliver) = global
1104+
.get(tc, key.into())
1105+
.and_then(|v| v8::Local::<v8::Function>::try_from(v).ok())
1106+
else {
1107+
return;
1108+
};
1109+
let id: v8::Local<v8::Value> = v8::Number::new(tc, worker_id as f64).into();
1110+
let _ = deliver.call(tc, global.into(), &[id]);
1111+
if tc.has_caught() {
1112+
if let Some(ex) = tc.exception() {
1113+
let msg = ex.to_rust_string_lossy(tc);
1114+
crate::store_last_js_error(msg);
1115+
}
1116+
tc.reset();
1117+
}
1118+
if !crate::defer_microtask_drain() {
1119+
tc.perform_microtask_checkpoint();
1120+
}
1121+
}
1122+
10831123
pub(crate) fn handle_worker_poll_messages(
10841124
scope: &mut v8::PinScope<'_, '_>,
10851125
args: v8::FunctionCallbackArguments,
@@ -4510,18 +4550,23 @@ pub(crate) unsafe extern "C" fn invoke_dotnet_js_callback(
45104550
_resp_len: *mut i32,
45114551
) {
45124552
let isolate_ptr = crate::DELEGATE_ISOLATE_PTR.with(|c| c.get());
4513-
if isolate_ptr.is_null() {
4553+
let owner = crate::dotnet_callback_owner(callback_id);
4554+
if isolate_ptr.is_null() || owner.is_some_and(|o| o != isolate_ptr) {
45144555
// A managed delegate, event, Task continuation or overridden member running on a thread
4515-
// other than the JS thread: hand the call to the JS (UI) thread and wait for it, so the
4516-
// callback runs and its result reaches the managed caller (see `run_on_ui_thread_sync`).
4517-
if !crate::ui_dispatcher::is_ui_thread() {
4518-
let (args, resp, resp_len) = (args_ptr as usize, _resp_ptr as usize, _resp_len as usize);
4519-
let delivered = crate::ui_dispatcher::run_on_ui_thread_sync(move || {
4520-
invoke_dotnet_js_callback(callback_id, args as *const u8, args_len, resp as *mut *mut u8, resp_len as *mut i32)
4521-
});
4522-
if delivered.is_none() {
4523-
write_callback_response(&[0x0F, 0, 0, 0, 0][..], _resp_ptr, _resp_len);
4524-
}
4556+
// other than the one its JS function belongs to (the UI thread or a worker): hand the call
4557+
// to that thread and wait for it, so the callback runs and its result reaches the managed
4558+
// caller (see `run_on_js_thread_sync`).
4559+
let (args, resp, resp_len) = (args_ptr as usize, _resp_ptr as usize, _resp_len as usize);
4560+
let call = move || {
4561+
invoke_dotnet_js_callback(callback_id, args as *const u8, args_len, resp as *mut *mut u8, resp_len as *mut i32)
4562+
};
4563+
let delivered = match owner {
4564+
Some(owner) => crate::run_on_js_thread_sync(owner, call),
4565+
None if !crate::ui_dispatcher::is_ui_thread() => crate::ui_dispatcher::run_on_ui_thread_sync(call),
4566+
None => None,
4567+
};
4568+
if delivered.is_none() {
4569+
write_callback_response(&[0x0F, 0, 0, 0, 0][..], _resp_ptr, _resp_len);
45254570
}
45264571
return;
45274572
}
@@ -4590,6 +4635,7 @@ pub(crate) unsafe extern "C" fn invoke_dotnet_js_callback(
45904635
crate::DOTNET_JS_CALLBACKS.with(|m| {
45914636
m.borrow_mut().remove(&callback_id);
45924637
});
4638+
crate::forget_dotnet_callback_owner(callback_id);
45934639
set.remove(&callback_id);
45944640
}
45954641
});
@@ -4620,18 +4666,43 @@ unsafe fn write_callback_response(bytes: &[u8], resp_ptr: *mut *mut u8, resp_len
46204666
}
46214667
}
46224668

4623-
/// Drops the JS functions whose managed delegates the .NET GC has collected.
4669+
/// Drops the JS functions whose managed delegates the .NET GC has collected. The release queue is
4670+
/// process-wide; a function that belongs to another JS thread (a worker's) is dropped there.
46244671
pub(crate) fn release_collected_js_callbacks() {
46254672
let released = crate::dotnet::take_released_js_callbacks();
46264673
if released.is_empty() {
46274674
return;
46284675
}
4676+
let current = crate::DELEGATE_ISOLATE_PTR.with(|c| c.get());
4677+
let mut elsewhere: HashMap<usize, Vec<i32>> = HashMap::new();
46294678
crate::DOTNET_JS_CALLBACKS.with(|m| {
46304679
let mut m = m.borrow_mut();
46314680
for id in released {
4632-
m.remove(&id);
4681+
match crate::dotnet_callback_owner(id) {
4682+
Some(owner) if owner != current => elsewhere.entry(owner as usize).or_default().push(id),
4683+
_ => {
4684+
m.remove(&id);
4685+
crate::forget_dotnet_callback_owner(id);
4686+
}
4687+
}
46334688
}
46344689
});
4690+
for (owner, ids) in elsewhere {
4691+
let owned = ids.clone();
4692+
let queued = crate::post_to_js_thread(owner as *mut v8::Isolate, move || {
4693+
crate::DOTNET_JS_CALLBACKS.with(|m| {
4694+
let mut m = m.borrow_mut();
4695+
for id in ids {
4696+
m.remove(&id);
4697+
crate::forget_dotnet_callback_owner(id);
4698+
}
4699+
});
4700+
});
4701+
if !queued {
4702+
// That runtime is gone and took its functions with it.
4703+
owned.into_iter().for_each(crate::forget_dotnet_callback_owner);
4704+
}
4705+
}
46354706
}
46364707

46374708
/// Registers `func` as a callback the bridge can invoke and returns its id.
@@ -4640,6 +4711,7 @@ pub(crate) fn register_dotnet_js_callback(scope: &mut v8::PinScope<'_, '_>, func
46404711
crate::DOTNET_JS_CALLBACKS.with(|m| {
46414712
m.borrow_mut().insert(cb_id, v8::Global::new(scope, func));
46424713
});
4714+
crate::record_dotnet_callback_owner(cb_id);
46434715
cb_id
46444716
}
46454717

@@ -4700,6 +4772,7 @@ pub(crate) fn handle_dotnet_create_delegate(
47004772
crate::DOTNET_JS_CALLBACKS.with(|m| {
47014773
m.borrow_mut().insert(cb_id, v8::Global::new(scope, func));
47024774
});
4775+
crate::record_dotnet_callback_owner(cb_id);
47034776

47044777
// opcode 0x09 | type_name (str16) | callback_id (i32)
47054778
let mut req: Vec<u8> = Vec::with_capacity(32);
@@ -4787,6 +4860,7 @@ pub(crate) fn handle_dotnet_create_js_subclass(
47874860
crate::DOTNET_JS_CALLBACKS.with(|m| {
47884861
m.borrow_mut().insert(cb_id, v8::Global::new(scope, cb_fn));
47894862
});
4863+
crate::record_dotnet_callback_owner(cb_id);
47904864

47914865
let mut req: Vec<u8> = Vec::with_capacity(64);
47924866
req.push(0x0A);
@@ -4871,6 +4945,8 @@ pub(crate) fn handle_dotnet_await_task(
48714945
map.insert(resolve_id, v8::Global::new(scope, resolve_fn));
48724946
map.insert(reject_id, v8::Global::new(scope, reject_fn));
48734947
});
4948+
crate::record_dotnet_callback_owner(resolve_id);
4949+
crate::record_dotnet_callback_owner(reject_id);
48744950

48754951
// Binary instance call: 0x01 | handle(i32) | "__dotnet_await__"(str16) | 2 | i32 resolveId | i32 rejectId
48764952
let mut req: Vec<u8> = Vec::with_capacity(32);
@@ -4889,6 +4965,8 @@ pub(crate) fn handle_dotnet_await_task(
48894965
map.remove(&resolve_id);
48904966
map.remove(&reject_id);
48914967
});
4968+
crate::forget_dotnet_callback_owner(resolve_id);
4969+
crate::forget_dotnet_callback_owner(reject_id);
48924970
throw_js_error(scope, &e);
48934971
}
48944972
}

‎runtime/src/lib.rs‎

Lines changed: 114 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,89 @@ pub fn pump_messages() -> bool {
374374
dispatched
375375
}
376376

377+
/// Runs `f` on the JS thread whose isolate is `owner` (the UI thread's or a worker's) and waits for
378+
/// its result: inline when that is the current thread, otherwise handed over the way a callback
379+
/// invoked on a background thread is. `None` when that thread can't run it (gone, or unknown).
380+
#[cfg(feature = "classic")]
381+
pub(crate) fn run_on_js_thread_sync<R: Send>(owner: *mut v8::Isolate, f: impl FnOnce() -> R + Send) -> Option<R> {
382+
if !is_live_isolate(owner) {
383+
return None;
384+
}
385+
if DELEGATE_ISOLATE_PTR.with(|c| c.get()) == owner {
386+
return Some(f());
387+
}
388+
if worker_threads::is_worker_isolate(owner as usize) {
389+
return worker_threads::run_on_worker_sync(owner as usize, f);
390+
}
391+
if ui_dispatcher::is_ui_thread() {
392+
return None;
393+
}
394+
ui_dispatcher::run_on_ui_thread_sync(f)
395+
}
396+
397+
/// Queues `f` on the JS thread whose isolate is `owner` without waiting; runs it inline when that
398+
/// is the current thread. `false` when there is no such thread to run it on.
399+
#[cfg(feature = "classic")]
400+
pub(crate) fn post_to_js_thread(owner: *mut v8::Isolate, f: impl FnOnce() + Send + 'static) -> bool {
401+
if !is_live_isolate(owner) {
402+
return false;
403+
}
404+
if DELEGATE_ISOLATE_PTR.with(|c| c.get()) == owner {
405+
f();
406+
return true;
407+
}
408+
if worker_threads::is_worker_isolate(owner as usize) {
409+
return worker_threads::post_to_worker(owner as usize, f);
410+
}
411+
if !ui_dispatcher::is_initialized() {
412+
return false;
413+
}
414+
ui_dispatcher::post_to_ui_thread(f);
415+
true
416+
}
417+
418+
/// The isolates of the runtimes alive in this process (the UI thread's and each worker's). A
419+
/// callback that outlives its worker can't run anywhere: work for a disposed isolate is refused.
420+
#[cfg(feature = "classic")]
421+
static LIVE_ISOLATES: std::sync::LazyLock<std::sync::Mutex<std::collections::HashSet<usize>>> =
422+
std::sync::LazyLock::new(|| std::sync::Mutex::new(std::collections::HashSet::new()));
423+
424+
#[cfg(feature = "classic")]
425+
fn is_live_isolate(isolate: *mut v8::Isolate) -> bool {
426+
!isolate.is_null() && LIVE_ISOLATES.lock().is_ok_and(|live| live.contains(&(isolate as usize)))
427+
}
428+
429+
/// The isolate each .NET callback's JS function belongs to (callback ids are process-wide, the
430+
/// functions live in their JS thread's `DOTNET_JS_CALLBACKS`), so a callback the bridge invokes on
431+
/// another thread is run on the right one.
432+
#[cfg(feature = "classic")]
433+
static DOTNET_CALLBACK_OWNERS: std::sync::LazyLock<std::sync::Mutex<HashMap<i32, usize>>> =
434+
std::sync::LazyLock::new(|| std::sync::Mutex::new(HashMap::new()));
435+
436+
#[cfg(feature = "classic")]
437+
pub(crate) fn record_dotnet_callback_owner(callback_id: i32) {
438+
let owner = DELEGATE_ISOLATE_PTR.with(|c| c.get()) as usize;
439+
if let Ok(mut owners) = DOTNET_CALLBACK_OWNERS.lock() {
440+
owners.insert(callback_id, owner);
441+
}
442+
}
443+
444+
#[cfg(feature = "classic")]
445+
pub(crate) fn dotnet_callback_owner(callback_id: i32) -> Option<*mut v8::Isolate> {
446+
DOTNET_CALLBACK_OWNERS
447+
.lock()
448+
.ok()
449+
.and_then(|owners| owners.get(&callback_id).copied())
450+
.map(|o| o as *mut v8::Isolate)
451+
}
452+
453+
#[cfg(feature = "classic")]
454+
pub(crate) fn forget_dotnet_callback_owner(callback_id: i32) {
455+
if let Ok(mut owners) = DOTNET_CALLBACK_OWNERS.lock() {
456+
owners.remove(&callback_id);
457+
}
458+
}
459+
377460
/// Retrieve (and clear) the last stored JS error.
378461
pub fn get_last_js_error() -> Option<String> {
379462
LAST_JS_ERROR.with(|e| e.borrow_mut().take())
@@ -8009,8 +8092,8 @@ pub(crate) struct JsDelegateData {
80098092
/// For a delegate whose Invoke the shared vtable can't implement (see [`delegate_invoke`]):
80108093
/// the typed Invoke and the delegate's own vtable, which points at it.
80118094
typed: Option<(delegate_invoke::TypedInvoke, Box<JsDelegateVtbl>)>,
8012-
/// The isolate `js_func` belongs to: the UI thread's, or null for a delegate created where no
8013-
/// isolate is registered (a worker), which can't be handed to the UI thread.
8095+
/// The isolate `js_func` belongs to (the UI thread's or a worker's): Invoke and the final
8096+
/// Release run on that isolate's thread.
80148097
owner_isolate: *mut v8::Isolate,
80158098
}
80168099

@@ -8090,15 +8173,21 @@ unsafe extern "system" fn js_delegate_release(this: *mut JsDelegate) -> u32 {
80908173
std::sync::atomic::fence(AtomicOrdering::Acquire);
80918174
let b = Box::from_raw(this);
80928175
let data = Box::from_raw(b.data);
8093-
// The function handle belongs to the UI thread's isolate: a final release on a background
8094-
// thread (the component that invoked the delegate there) frees it on the UI thread.
8095-
if !data.owner_isolate.is_null() && !ui_dispatcher::is_ui_thread() && ui_dispatcher::is_initialized() {
8096-
struct SendData(Box<JsDelegateData>);
8097-
unsafe impl Send for SendData {}
8098-
let data = SendData(data);
8099-
ui_dispatcher::post_to_ui_thread(move || drop(data));
8100-
} else {
8176+
// The function handle belongs to its JS thread's isolate: a final release on another thread
8177+
// (the component that invoked the delegate there) frees it on that thread.
8178+
let owner = data.owner_isolate;
8179+
if DELEGATE_ISOLATE_PTR.with(|c| c.get()) == owner || owner.is_null() {
81018180
drop(data);
8181+
} else {
8182+
// Never dropped elsewhere: if that thread is gone (a terminated worker), the handle
8183+
// leaks rather than being freed into a disposed isolate.
8184+
struct SendData(std::mem::ManuallyDrop<Box<JsDelegateData>>);
8185+
unsafe impl Send for SendData {}
8186+
let data = SendData(std::mem::ManuallyDrop::new(data));
8187+
post_to_js_thread(owner, move || {
8188+
let data = data;
8189+
drop(std::mem::ManuallyDrop::into_inner(data.0));
8190+
});
81028191
}
81038192
// b (JsDelegate) dropped here
81048193
}
@@ -8183,23 +8272,16 @@ fn js_delegate_invoke_with_args(
81838272
result_slot: *mut c_void,
81848273
) -> HRESULT {
81858274
let isolate_ptr = DELEGATE_ISOLATE_PTR.with(|c| c.get());
8186-
if !isolate_ptr.is_null() && isolate_ptr != data.owner_isolate {
8187-
// Never run a function against an isolate it doesn't belong to.
8188-
return HRESULT(0x8001010Eu32 as i32); // RPC_E_WRONG_THREAD
8189-
}
8190-
if isolate_ptr.is_null() {
8191-
// Invoked directly on a background thread (a component that calls its handlers without
8192-
// marshaling them back to the registering apartment): run the JS on the JS (UI) thread
8193-
// and wait, so the call is delivered and its result written before Invoke returns. The
8194-
// arguments are borrowed for the duration of Invoke, which is exactly how long they're used.
8195-
// Only the UI thread's functions can go there; a worker's stay undeliverable.
8196-
if ui_dispatcher::is_ui_thread() || data.owner_isolate.is_null() {
8197-
return HRESULT(0x80004005u32 as i32);
8198-
}
8275+
if isolate_ptr != data.owner_isolate {
8276+
// Invoked on another thread (a component that calls its handlers without marshaling them
8277+
// back to the registering apartment): run the JS on the thread that owns the function —
8278+
// the UI thread or a worker — and wait, so the call is delivered and its result written
8279+
// before Invoke returns. The arguments are borrowed for the duration of Invoke, which is
8280+
// exactly how long they're used.
81998281
struct SendPtr(*mut JsDelegate, *mut c_void);
82008282
unsafe impl Send for SendPtr {}
82018283
let ptrs = SendPtr(this, result_slot);
8202-
return ui_dispatcher::run_on_ui_thread_sync(move || {
8284+
return run_on_js_thread_sync(data.owner_isolate, move || {
82038285
let ptrs = ptrs;
82048286
// SAFETY: the caller holds a reference on the delegate for the duration of Invoke.
82058287
let data = unsafe { &*(*ptrs.0).data };
@@ -9036,6 +9118,9 @@ impl Runtime {
90369118
pub fn register_delegate_isolate_ptr(&mut self) {
90379119
let raw_isolate: *mut v8::Isolate = &mut *self.isolate as *mut v8::Isolate;
90389120
DELEGATE_ISOLATE_PTR.with(|cell| cell.set(raw_isolate));
9121+
if let Ok(mut live) = LIVE_ISOLATES.lock() {
9122+
live.insert(raw_isolate as usize);
9123+
}
90399124
}
90409125

90419126
/// Provides mutable access to the underlying V8 isolate.
@@ -9352,6 +9437,9 @@ impl Drop for Runtime {
93529437
// raw isolate pointers) must be cleared here, while `self.isolate` is
93539438
// still alive. Anything left behind dangles into freed isolate memory
93549439
// and crashes the next Runtime created on this thread.
9440+
if let Ok(mut live) = LIVE_ISOLATES.lock() {
9441+
live.remove(&(&mut *self.isolate as *mut v8::Isolate as usize));
9442+
}
93559443
crate::wrapper_cache::clear();
93569444
EVENT_REGISTRY.with(|m| m.borrow_mut().clear());
93579445
ESM_MODULE_REGISTRY.with(|m| m.borrow_mut().clear());
@@ -9360,6 +9448,8 @@ impl Drop for Runtime {
93609448
ESM_HTTP_URLS.with(|m| m.borrow_mut().clear());
93619449
ESM_LOAD_ERRORS.with(|m| m.borrow_mut().clear());
93629450
crate::esm_http::clear_thread_vocabulary();
9451+
// The ids keep their owner: a managed delegate that outlives this runtime (a terminated
9452+
// worker's) is refused rather than run elsewhere, until .NET releases it.
93639453
DOTNET_JS_CALLBACKS.with(|m| m.borrow_mut().clear());
93649454
DOTNET_ONESHOT_JS_CALLBACKS.with(|m| m.borrow_mut().clear());
93659455
crate::timers::clear_thread_tasks();

0 commit comments

Comments
 (0)