Skip to content

Commit 5fe27d7

Browse files
use instance pool for call_view
1 parent e2beffa commit 5fe27d7

2 files changed

Lines changed: 66 additions & 7 deletions

File tree

crates/core/src/host/module_host.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,6 +1279,7 @@ impl ModuleHost {
12791279
.await
12801280
},
12811281
async move |timer_guard, inst, arg| {
1282+
super::v8::assert_not_on_js_module_thread(label);
12821283
drop(timer_guard);
12831284
js(arg, inst).await
12841285
},
@@ -1316,13 +1317,21 @@ impl ModuleHost {
13161317
}
13171318

13181319
async fn call_view_command(&self, label: &str, cmd: ViewCommand) -> Result<ViewCommandResult, ViewCallError> {
1319-
self.call(
1320-
label,
1321-
cmd,
1322-
async |cmd, inst| Ok(inst.call_view(cmd)),
1323-
async |cmd, inst| inst.call_view(cmd).await,
1324-
)
1325-
.await?
1320+
Ok(match &*self.inner {
1321+
ModuleHostInner::Wasm(_) => {
1322+
self.call(
1323+
label,
1324+
cmd,
1325+
async |cmd, inst| Ok::<_, ViewCallError>(inst.call_view(cmd)),
1326+
async |_cmd, _inst| unreachable!("WASM should not use the JS call_view path"),
1327+
)
1328+
.await??
1329+
}
1330+
ModuleHostInner::Js(_) => {
1331+
self.with_js_pooled_instance(label, async |inst| inst.call_view(cmd).await)
1332+
.await?
1333+
}
1334+
})
13261335
}
13271336

13281337
pub async fn disconnect_client(&self, client_id: ClientActorId) {

crates/core/src/host/v8/mod.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ use spacetimedb_schema::auto_migrate::MigrationPolicy;
4646
use spacetimedb_schema::def::ModuleDef;
4747
use spacetimedb_schema::identifier::Identifier;
4848
use spacetimedb_table::static_assert_size;
49+
use std::cell::Cell;
4950
use std::panic::AssertUnwindSafe;
5051
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
5152
use std::sync::{Arc, LazyLock};
@@ -96,6 +97,52 @@ impl V8Runtime {
9697
static V8_RUNTIME_GLOBAL: LazyLock<V8RuntimeInner> = LazyLock::new(V8RuntimeInner::init);
9798
static NEXT_JS_INSTANCE_ID: AtomicU64 = AtomicU64::new(1);
9899

100+
thread_local! {
101+
// Note, `on_module_thread` runs host closures on a single JS module thread.
102+
// Enqueuing more JS module-thread work from one of those closures waits on the
103+
// same worker thread that is already busy running the current closure.
104+
// And this deadlocks.
105+
static ON_JS_MODULE_THREAD: Cell<bool> = const { Cell::new(false) };
106+
}
107+
108+
struct EnteredJsModuleThread;
109+
110+
impl EnteredJsModuleThread {
111+
fn new() -> Self {
112+
ON_JS_MODULE_THREAD.with(|entered| {
113+
assert!(
114+
!entered.get(),
115+
"reentrancy into the JS module thread; this would deadlock. \
116+
Do not enqueue onto this worker from inside `on_module_thread` work."
117+
);
118+
entered.set(true);
119+
});
120+
Self
121+
}
122+
}
123+
124+
impl Drop for EnteredJsModuleThread {
125+
fn drop(&mut self) {
126+
ON_JS_MODULE_THREAD.with(|entered| {
127+
debug_assert!(
128+
entered.get(),
129+
"JS module thread marker should only be cleared after entry"
130+
);
131+
entered.set(false);
132+
});
133+
}
134+
}
135+
136+
pub(crate) fn assert_not_on_js_module_thread(label: &str) {
137+
ON_JS_MODULE_THREAD.with(|entered| {
138+
assert!(
139+
!entered.get(),
140+
"{label} attempted to re-enter the JS module thread from code already \
141+
running on that thread; this would deadlock"
142+
);
143+
});
144+
}
145+
99146
/// The actual V8 runtime, with initialization of V8.
100147
struct V8RuntimeInner {
101148
_priv: (),
@@ -604,6 +651,8 @@ impl JsInstanceLane {
604651
label: &'static str,
605652
work: impl AsyncFnOnce(JsInstance) -> Result<R, WorkerDisconnected>,
606653
) -> Result<R, WorkerDisconnected> {
654+
assert_not_on_js_module_thread(label);
655+
607656
let active = self.active_instance();
608657
let result = work(active.clone()).await;
609658
match result {
@@ -636,6 +685,7 @@ impl JsInstanceLane {
636685
inst.request_tx
637686
.send_async(JsWorkerRequest::RunFunction(Box::new(move || {
638687
async move {
688+
let _on_js_module_thread = EnteredJsModuleThread::new();
639689
let result = AssertUnwindSafe(f().instrument(span)).catch_unwind().await;
640690
if let Err(Err(_panic)) = tx.send(result) {
641691
tracing::warn!("uncaught panic on `SingleCoreExecutor`")

0 commit comments

Comments
 (0)