fix: dispose unsettled bridge deferreds before tearing down the tag sandbox - #10403
Open
jackkav wants to merge 1 commit into
Open
fix: dispose unsettled bridge deferreds before tearing down the tag sandbox#10403jackkav wants to merge 1 commit into
jackkav wants to merge 1 commit into
Conversation
…andbox
`installHostBridge` created a `ctx.newPromise()` per `__hostBridge` call and
returned `deferred.handle` without ever disposing the deferred. Each
`QuickJSDeferredPromise` owns three JSValues — the promise plus its
`resolve`/`reject` function handles — and only `resolve()`/`reject()` frees the
two resolvers.
So when a bridge call had not settled by the time the run ended, those resolvers
were still live and `ctx.dispose()` aborted the whole WASM module:
Aborted(Assertion failed: list_empty(&rt->gc_obj_list), at: quickjs.c,2036,JS_FreeRuntime)
An abort is unrecoverable — it takes down the runtime rather than surfacing a
catchable error — and it was reachable two ways: the 10s deadline firing while a
slow `network.sendRequest`/nested `util.render` was outstanding, and a tag
unwinding via `__task` rejection while a sibling await was still in flight.
`installHostBridge` now tracks its deferreds and returns a teardown function
that `runTagInSandbox`'s `finally` runs before `ctx.dispose()`.
Teardown frees the resolvers silently instead of settling them. Settling would
resume the sandbox mid-teardown and settle `__task`, at which point
`ctx.resolvePromise`'s reject callback dup()s the error into a host promise
nobody awaits — leaking that handle and aborting the same way, one step removed.
For the same reason teardown must not pump `executePendingJobs()`.
Two liveness guards cover the late-settle race: `resolveWithString` no-ops on a
dead context or deferred, and the `deferred.settled` job pump is gated on
`ctx.alive`, so a bridge answering after teardown does not raise
`QuickJSUseAfterFree` as an unhandled rejection.
✅ Circular References ReportGenerated at: 2026-08-14T15:12:50.835Z Summary
Click to view all circular references in PR (10)Click to view all circular references in base branch (10)Analysis✅ No Change: This PR does not introduce or remove any circular references. This report was generated automatically by comparing against the |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a QuickJS runtime abort in the template-tag sandbox caused by unsettled ctx.newPromise() deferreds created by __hostBridge calls that never resolve/reject before teardown. The change ensures pending bridge deferreds are disposed before ctx.dispose() so QuickJS doesn’t abort on live GC objects at runtime free.
Changes:
- Track all outstanding
__hostBridgedeferreds and return a teardown function that disposes any still-pending deferreds prior to context disposal. - Add liveness guards so late-arriving bridge resolutions don’t attempt to allocate/resolve into a torn-down QuickJS context.
- Add a regression test that reproduces the timeout-with-in-flight-bridge scenario and verifies it times out cleanly without aborting.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/insomnia/src/templating/sandbox/plugin-tag-sandbox.ts | Track and dispose pending host-bridge deferred promises before ctx.dispose(), and guard late-settle/job-pumping on context/deferred liveness. |
| packages/insomnia/src/templating/sandbox/plugin-tag-sandbox.test.ts | Adds a regression test ensuring timeouts with an in-flight host bridge call do not abort the QuickJS runtime and late settle is a no-op. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
installHostBridgeinplugin-tag-sandbox.tscreates actx.newPromise()per__hostBridgecall and returnsdeferred.handle, but never disposes the deferred.A
QuickJSDeferredPromiseowns three JSValues — the promise plus itsresolve/rejectfunction handles — and onlyresolve()/reject()frees the two resolvers (via the privatedisposeResolvers()). The library's own docs are explicit that returninghandlefrom aVmFunctionImplementationis safe only if you "ensure that either resolve or reject will be called", and that you must calldispose()otherwise.We didn't. So when a bridge call hadn't settled by the time the run ended, the resolvers were still live and
ctx.dispose()aborted the WASM module:This is an abort, not an exception — it kills the runtime instead of surfacing a catchable error, so the caller never sees the timeout it was supposed to get.
Two reachable paths, both with real plugins:
network.sendRequestor nestedutil.renderis outstanding.__taskrejection while a sibling await is still in flight — e.g.Promise.all([slowBridgeCall(), somethingThatThrows()]).Present on
developin shipped code; not introduced by any open PR.Fix
installHostBridgenow tracks its deferreds and returns a teardown function thatrunTagInSandbox'sfinallyruns beforectx.dispose().Teardown frees the resolvers silently rather than settling them, and deliberately does not pump
executePendingJobs(). Settling during teardown resumes the sandbox mid-unwind and settles__task— at which pointctx.resolvePromise's reject callbackdup()s the error into a host promise nobody is listening to, leaking that handle and aborting in exactly the same way, one step removed.dispose()is idempotent and guards each handle, so the abandoned VM promise just stays pending for the microsecond before the runtime goes away.Two liveness guards cover the late-settle race, so a bridge answering after teardown is a no-op instead of a
QuickJSUseAfterFreeunhandled rejection:resolveWithStringbails on!ctx.alive || !deferred.alivebefore allocating anything.deferred.settledjob pump is gated onctx.alive.Verification
The regression test reproduces the abort against the real
runTagInSandbox— it fails with thelist_empty(&rt->gc_obj_list)abort before the fix and passes after, with the late bridge response arriving post-teardown to exercise the guards.plugin-tag-sandbox.test.ts: 86/86 passsrc/templating+src/scripting: 490/490 passtype-checkand ESLint cleanMechanism was confirmed against the shipped
quickjs-emscripten-coresource, not just the docs:resolve/rejectare the only callers ofdisposeResolvers(),settledresolves only fromonSettled()inside those two, andresolvePromisemanages its callback handles in aScope(so it leaks nothing on the timeout path as long as__taskstays unsettled during unwind).Note
The same bug exists in
packages/insomnia/src/scripting/quickjs-script-engine.tsand is what #10392 is stuck on. Not touched here — this PR is scoped to the templating sandbox.