Skip to content

Commit a08f6c4

Browse files
simongdaviessimongdavies
committed
feat: add explicit dispose() methods to JSSandbox and LoadedJSSandbox NAPI wrappers
Add #[napi] dispose() methods that eagerly release underlying sandbox resources by calling take() on the inner Option. After disposal, all subsequent calls return ERR_CONSUMED. No-op on already-consumed instances. Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
1 parent a892a0a commit a08f6c4

File tree

3 files changed

+211
-12
lines changed

3 files changed

+211
-12
lines changed

‎src/js-host-api/lib.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,18 +144,19 @@ function wrapGetter(cls, prop) {
144144
}
145145

146146
// LoadedJSSandbox — async methods
147-
// Note: `poisoned` (AtomicBool read) and `interruptHandle` (Arc clone)
148-
// are infallible getters — no wrapping needed.
149-
for (const method of ['callHandler', 'unload', 'snapshot', 'restore']) {
147+
for (const method of ['callHandler', 'unload', 'snapshot', 'restore', 'dispose']) {
150148
const orig = LoadedJSSandbox.prototype[method];
151149
if (!orig) throw new Error(`Cannot wrap missing method: LoadedJSSandbox.${method}`);
152150
LoadedJSSandbox.prototype[method] = wrapAsync(orig);
153151
}
152+
wrapGetter(LoadedJSSandbox, 'poisoned');
153+
wrapGetter(LoadedJSSandbox, 'interruptHandle');
154+
wrapGetter(LoadedJSSandbox, 'lastCallStats');
154155

155156
// JSSandbox — async + sync methods + getters
156157
JSSandbox.prototype.getLoadedSandbox = wrapAsync(JSSandbox.prototype.getLoadedSandbox);
157158

158-
for (const method of ['addHandler', 'removeHandler', 'clearHandlers']) {
159+
for (const method of ['addHandler', 'removeHandler', 'clearHandlers', 'dispose']) {
159160
const orig = JSSandbox.prototype[method];
160161
if (!orig) throw new Error(`Cannot wrap missing method: JSSandbox.${method}`);
161162
JSSandbox.prototype[method] = wrapSync(orig);

‎src/js-host-api/src/lib.rs‎

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,7 @@ impl JSSandboxWrapper {
790790
interrupt,
791791
poisoned_flag,
792792
last_call_stats: Arc::new(ArcSwapOption::empty()),
793+
disposed_flag: Arc::new(AtomicBool::new(false)),
793794
})
794795
}
795796

@@ -801,6 +802,20 @@ impl JSSandboxWrapper {
801802
pub fn poisoned(&self) -> napi::Result<bool> {
802803
self.with_inner_ref(|sandbox| Ok(sandbox.poisoned()))
803804
}
805+
806+
/// Eagerly release the underlying sandbox resources.
807+
///
808+
/// After calling `dispose()`, the sandbox is consumed and all
809+
/// subsequent method calls will throw an `ERR_CONSUMED` error.
810+
/// This is useful when you want deterministic cleanup rather than
811+
/// waiting for garbage collection.
812+
///
813+
/// Calling `dispose()` on an already-consumed sandbox is a no-op.
814+
#[napi]
815+
pub fn dispose(&self) -> napi::Result<()> {
816+
let _ = self.inner.lock().map_err(|_| lock_error())?.take();
817+
Ok(())
818+
}
804819
}
805820

806821
// ── LoadedJSSandbox ──────────────────────────────────────────────────
@@ -853,6 +868,11 @@ pub struct LoadedJSSandboxWrapper {
853868
/// closures (which require `'static + Send`). `ArcSwapOption` alone is
854869
/// not `Clone` — the `Arc` provides cheap shared ownership across threads.
855870
last_call_stats: Arc<ArcSwapOption<CallStats>>,
871+
872+
/// Tracks whether this wrapper has been consumed (via `dispose()` or
873+
/// `unload()`), for lock-free checks in sync getters that don't
874+
/// consult the inner Mutex.
875+
disposed_flag: Arc<AtomicBool>,
856876
}
857877

858878
#[napi]
@@ -1013,11 +1033,13 @@ impl LoadedJSSandboxWrapper {
10131033
#[napi]
10141034
pub async fn unload(&self) -> napi::Result<JSSandboxWrapper> {
10151035
let inner = self.inner.clone();
1036+
let disposed = self.disposed_flag.clone();
10161037
let js_sandbox = tokio::task::spawn_blocking(move || {
10171038
let mut guard = inner.lock().map_err(|_| lock_error())?;
10181039
let loaded = guard
10191040
.take()
10201041
.ok_or_else(|| consumed_error("LoadedJSSandbox"))?;
1042+
disposed.store(true, Ordering::Release);
10211043
loaded.unload().map_err(to_napi_error)
10221044
})
10231045
.await
@@ -1043,11 +1065,15 @@ impl LoadedJSSandboxWrapper {
10431065
/// the sandbox lock — it is always available instantly.
10441066
///
10451067
/// @returns An `InterruptHandle` with a `kill()` method
1068+
/// @throws `ERR_CONSUMED` if the sandbox has been consumed via `dispose()` or `unload()`
10461069
#[napi(getter)]
1047-
pub fn interrupt_handle(&self) -> InterruptHandleWrapper {
1048-
InterruptHandleWrapper {
1049-
inner: self.interrupt.clone(),
1070+
pub fn interrupt_handle(&self) -> napi::Result<InterruptHandleWrapper> {
1071+
if self.disposed_flag.load(Ordering::Acquire) {
1072+
return Err(consumed_error("LoadedJSSandbox"));
10501073
}
1074+
Ok(InterruptHandleWrapper {
1075+
inner: self.interrupt.clone(),
1076+
})
10511077
}
10521078

10531079
/// Whether the sandbox is in a poisoned (inconsistent) state.
@@ -1063,9 +1089,14 @@ impl LoadedJSSandboxWrapper {
10631089
/// Recovery options:
10641090
/// - `restore(snapshot)` — revert to a captured state
10651091
/// - `unload()` — discard handlers and start fresh
1092+
///
1093+
/// @throws `ERR_CONSUMED` if the sandbox has been consumed via `dispose()` or `unload()`
10661094
#[napi(getter)]
1067-
pub fn poisoned(&self) -> bool {
1068-
self.poisoned_flag.load(Ordering::Acquire)
1095+
pub fn poisoned(&self) -> napi::Result<bool> {
1096+
if self.disposed_flag.load(Ordering::Acquire) {
1097+
return Err(consumed_error("LoadedJSSandbox"));
1098+
}
1099+
Ok(self.poisoned_flag.load(Ordering::Acquire))
10691100
}
10701101

10711102
/// Execution statistics from the most recent `callHandler()` call.
@@ -1089,12 +1120,18 @@ impl LoadedJSSandboxWrapper {
10891120
/// if (stats.terminatedBy) console.log(`Killed by: ${stats.terminatedBy}`);
10901121
/// }
10911122
/// ```
1123+
///
1124+
/// @throws `ERR_CONSUMED` if the sandbox has been consumed via `dispose()` or `unload()`
10921125
#[napi(getter)]
1093-
pub fn last_call_stats(&self) -> Option<CallStats> {
1094-
self.last_call_stats
1126+
pub fn last_call_stats(&self) -> napi::Result<Option<CallStats>> {
1127+
if self.disposed_flag.load(Ordering::Acquire) {
1128+
return Err(consumed_error("LoadedJSSandbox"));
1129+
}
1130+
Ok(self
1131+
.last_call_stats
10951132
.load()
10961133
.as_ref()
1097-
.map(|arc| (**arc).clone())
1134+
.map(|arc| (**arc).clone()))
10981135
}
10991136

11001137
/// Capture the current sandbox state as a snapshot.
@@ -1150,6 +1187,30 @@ impl LoadedJSSandboxWrapper {
11501187
.await
11511188
.map_err(join_error)?
11521189
}
1190+
1191+
/// Eagerly release the underlying sandbox resources.
1192+
///
1193+
/// After calling `dispose()`, the sandbox is consumed and all
1194+
/// subsequent method calls will throw an `ERR_CONSUMED` error.
1195+
/// This is useful when you want deterministic cleanup rather than
1196+
/// waiting for garbage collection.
1197+
///
1198+
/// Calling `dispose()` on an already-consumed sandbox is a no-op.
1199+
#[napi]
1200+
pub async fn dispose(&self) -> napi::Result<()> {
1201+
if self.disposed_flag.load(Ordering::Acquire) {
1202+
return Ok(());
1203+
}
1204+
let inner = self.inner.clone();
1205+
let disposed = self.disposed_flag.clone();
1206+
tokio::task::spawn_blocking(move || {
1207+
let _ = inner.lock().map_err(|_| lock_error())?.take();
1208+
disposed.store(true, Ordering::Release);
1209+
Ok(())
1210+
})
1211+
.await
1212+
.map_err(join_error)?
1213+
}
11531214
}
11541215

11551216
// ── CallHandlerOptions ───────────────────────────────────────────────

‎src/js-host-api/tests/sandbox.test.js‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,3 +308,140 @@ describe('Calculator example', () => {
308308
expect(result.result).toBe(4);
309309
});
310310
});
311+
312+
// ── dispose() ────────────────────────────────────────────────────────
313+
314+
describe('JSSandbox.dispose()', () => {
315+
it('should make subsequent method calls throw ERR_CONSUMED', async () => {
316+
const builder = new SandboxBuilder();
317+
const proto = await builder.build();
318+
const sandbox = await proto.loadRuntime();
319+
320+
sandbox.dispose();
321+
322+
expectThrowsWithCode(
323+
() => sandbox.addHandler('h', 'function handler() {}'),
324+
'ERR_CONSUMED'
325+
);
326+
});
327+
328+
it('should be a no-op when called twice', async () => {
329+
const builder = new SandboxBuilder();
330+
const proto = await builder.build();
331+
const sandbox = await proto.loadRuntime();
332+
333+
sandbox.dispose();
334+
sandbox.dispose(); // should not throw
335+
});
336+
337+
it('should make poisoned getter throw ERR_CONSUMED', async () => {
338+
const builder = new SandboxBuilder();
339+
const proto = await builder.build();
340+
const sandbox = await proto.loadRuntime();
341+
342+
sandbox.dispose();
343+
344+
expectThrowsWithCode(() => sandbox.poisoned, 'ERR_CONSUMED');
345+
});
346+
});
347+
348+
describe('LoadedJSSandbox.dispose()', () => {
349+
it('should make subsequent callHandler reject with ERR_CONSUMED', async () => {
350+
const builder = new SandboxBuilder();
351+
const proto = await builder.build();
352+
const sandbox = await proto.loadRuntime();
353+
sandbox.addHandler('handler', 'function handler() { return { ok: true }; }');
354+
const loaded = await sandbox.getLoadedSandbox();
355+
356+
await loaded.dispose();
357+
358+
await expectRejectsWithCode(loaded.callHandler('handler', {}), 'ERR_CONSUMED');
359+
});
360+
361+
it('should be a no-op when called twice', async () => {
362+
const builder = new SandboxBuilder();
363+
const proto = await builder.build();
364+
const sandbox = await proto.loadRuntime();
365+
sandbox.addHandler('handler', 'function handler() { return {}; }');
366+
const loaded = await sandbox.getLoadedSandbox();
367+
368+
await loaded.dispose();
369+
await loaded.dispose(); // should not throw
370+
});
371+
372+
it('should make poisoned getter throw ERR_CONSUMED', async () => {
373+
const builder = new SandboxBuilder();
374+
const proto = await builder.build();
375+
const sandbox = await proto.loadRuntime();
376+
sandbox.addHandler('handler', 'function handler() { return {}; }');
377+
const loaded = await sandbox.getLoadedSandbox();
378+
379+
await loaded.dispose();
380+
381+
expectThrowsWithCode(() => loaded.poisoned, 'ERR_CONSUMED');
382+
});
383+
384+
it('should make interruptHandle getter throw ERR_CONSUMED', async () => {
385+
const builder = new SandboxBuilder();
386+
const proto = await builder.build();
387+
const sandbox = await proto.loadRuntime();
388+
sandbox.addHandler('handler', 'function handler() { return {}; }');
389+
const loaded = await sandbox.getLoadedSandbox();
390+
391+
await loaded.dispose();
392+
393+
expectThrowsWithCode(() => loaded.interruptHandle, 'ERR_CONSUMED');
394+
});
395+
396+
it('should make lastCallStats getter throw ERR_CONSUMED', async () => {
397+
const builder = new SandboxBuilder();
398+
const proto = await builder.build();
399+
const sandbox = await proto.loadRuntime();
400+
sandbox.addHandler('handler', 'function handler() { return {}; }');
401+
const loaded = await sandbox.getLoadedSandbox();
402+
403+
await loaded.dispose();
404+
405+
expectThrowsWithCode(() => loaded.lastCallStats, 'ERR_CONSUMED');
406+
});
407+
});
408+
409+
// ── unload() consumption ─────────────────────────────────────────────
410+
411+
describe('LoadedJSSandbox.unload()', () => {
412+
it('should make poisoned getter throw ERR_CONSUMED after unload', async () => {
413+
const builder = new SandboxBuilder();
414+
const proto = await builder.build();
415+
const sandbox = await proto.loadRuntime();
416+
sandbox.addHandler('handler', 'function handler() { return {}; }');
417+
const loaded = await sandbox.getLoadedSandbox();
418+
419+
await loaded.unload();
420+
421+
expectThrowsWithCode(() => loaded.poisoned, 'ERR_CONSUMED');
422+
});
423+
424+
it('should make interruptHandle getter throw ERR_CONSUMED after unload', async () => {
425+
const builder = new SandboxBuilder();
426+
const proto = await builder.build();
427+
const sandbox = await proto.loadRuntime();
428+
sandbox.addHandler('handler', 'function handler() { return {}; }');
429+
const loaded = await sandbox.getLoadedSandbox();
430+
431+
await loaded.unload();
432+
433+
expectThrowsWithCode(() => loaded.interruptHandle, 'ERR_CONSUMED');
434+
});
435+
436+
it('should make lastCallStats getter throw ERR_CONSUMED after unload', async () => {
437+
const builder = new SandboxBuilder();
438+
const proto = await builder.build();
439+
const sandbox = await proto.loadRuntime();
440+
sandbox.addHandler('handler', 'function handler() { return {}; }');
441+
const loaded = await sandbox.getLoadedSandbox();
442+
443+
await loaded.unload();
444+
445+
expectThrowsWithCode(() => loaded.lastCallStats, 'ERR_CONSUMED');
446+
});
447+
});

0 commit comments

Comments
 (0)