Skip to content

Commit 92ea9dd

Browse files
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 92ea9dd

3 files changed

Lines changed: 160 additions & 12 deletions

File tree

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: 58 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,10 @@ 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 `dispose()` has been called, for lock-free checks
873+
/// in sync getters that don't consult the inner Mutex.
874+
disposed_flag: Arc<AtomicBool>,
856875
}
857876

858877
#[napi]
@@ -1044,10 +1063,13 @@ impl LoadedJSSandboxWrapper {
10441063
///
10451064
/// @returns An `InterruptHandle` with a `kill()` method
10461065
#[napi(getter)]
1047-
pub fn interrupt_handle(&self) -> InterruptHandleWrapper {
1048-
InterruptHandleWrapper {
1049-
inner: self.interrupt.clone(),
1066+
pub fn interrupt_handle(&self) -> napi::Result<InterruptHandleWrapper> {
1067+
if self.disposed_flag.load(Ordering::Acquire) {
1068+
return Err(consumed_error("LoadedJSSandbox"));
10501069
}
1070+
Ok(InterruptHandleWrapper {
1071+
inner: self.interrupt.clone(),
1072+
})
10511073
}
10521074

10531075
/// Whether the sandbox is in a poisoned (inconsistent) state.
@@ -1064,8 +1086,11 @@ impl LoadedJSSandboxWrapper {
10641086
/// - `restore(snapshot)` — revert to a captured state
10651087
/// - `unload()` — discard handlers and start fresh
10661088
#[napi(getter)]
1067-
pub fn poisoned(&self) -> bool {
1068-
self.poisoned_flag.load(Ordering::Acquire)
1089+
pub fn poisoned(&self) -> napi::Result<bool> {
1090+
if self.disposed_flag.load(Ordering::Acquire) {
1091+
return Err(consumed_error("LoadedJSSandbox"));
1092+
}
1093+
Ok(self.poisoned_flag.load(Ordering::Acquire))
10691094
}
10701095

10711096
/// Execution statistics from the most recent `callHandler()` call.
@@ -1090,11 +1115,15 @@ impl LoadedJSSandboxWrapper {
10901115
/// }
10911116
/// ```
10921117
#[napi(getter)]
1093-
pub fn last_call_stats(&self) -> Option<CallStats> {
1094-
self.last_call_stats
1118+
pub fn last_call_stats(&self) -> napi::Result<Option<CallStats>> {
1119+
if self.disposed_flag.load(Ordering::Acquire) {
1120+
return Err(consumed_error("LoadedJSSandbox"));
1121+
}
1122+
Ok(self
1123+
.last_call_stats
10951124
.load()
10961125
.as_ref()
1097-
.map(|arc| (**arc).clone())
1126+
.map(|arc| (**arc).clone()))
10981127
}
10991128

11001129
/// Capture the current sandbox state as a snapshot.
@@ -1150,6 +1179,27 @@ impl LoadedJSSandboxWrapper {
11501179
.await
11511180
.map_err(join_error)?
11521181
}
1182+
1183+
/// Eagerly release the underlying sandbox resources.
1184+
///
1185+
/// After calling `dispose()`, the sandbox is consumed and all
1186+
/// subsequent method calls will throw an `ERR_CONSUMED` error.
1187+
/// This is useful when you want deterministic cleanup rather than
1188+
/// waiting for garbage collection.
1189+
///
1190+
/// Calling `dispose()` on an already-consumed sandbox is a no-op.
1191+
#[napi]
1192+
pub async fn dispose(&self) -> napi::Result<()> {
1193+
let inner = self.inner.clone();
1194+
let disposed = self.disposed_flag.clone();
1195+
tokio::task::spawn_blocking(move || {
1196+
let _ = inner.lock().map_err(|_| lock_error())?.take();
1197+
disposed.store(true, Ordering::Release);
1198+
Ok(())
1199+
})
1200+
.await
1201+
.map_err(join_error)?
1202+
}
11531203
}
11541204

11551205
// ── CallHandlerOptions ───────────────────────────────────────────────

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,3 +308,100 @@ 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+
});

0 commit comments

Comments
 (0)