From bad7956cfb098d99a60eb98bf6b6dcf9cd72b9df Mon Sep 17 00:00:00 2001 From: Abhishek Patil Date: Wed, 17 Jun 2026 13:46:14 +0530 Subject: [PATCH] fix(perf):quickjs-memory-leak (#8219) --- .../bruno-js/src/sandbox/quickjs/index.js | 29 +- .../bruno-js/src/sandbox/quickjs/shims/bru.js | 4 + .../src/sandbox/quickjs/utils/index.js | 132 +++++++- .../tests/quickjs-handle-lifecycle.spec.js | 307 ++++++++++++++++++ 4 files changed, 464 insertions(+), 8 deletions(-) create mode 100644 packages/bruno-js/tests/quickjs-handle-lifecycle.spec.js diff --git a/packages/bruno-js/src/sandbox/quickjs/index.js b/packages/bruno-js/src/sandbox/quickjs/index.js index a5e1b84df..e22a561ce 100644 --- a/packages/bruno-js/src/sandbox/quickjs/index.js +++ b/packages/bruno-js/src/sandbox/quickjs/index.js @@ -11,7 +11,7 @@ const { newQuickJSWASMModule, memoizePromiseFactory } = require('quickjs-emscrip // execute `npm run sandbox:bundle-libraries` if the below file doesn't exist const getBundledCode = require('../bundle-browser-rollup'); const addPathShimToContext = require('./shims/lib/path'); -const { marshallToVm } = require('./utils'); +const { marshallToVm, createManagedQuickJsContext } = require('./utils'); const addCryptoUtilsShimToContext = require('./shims/lib/crypto-utils'); const { wrapScriptInClosure, SANDBOX } = require('../../utils/sandbox'); @@ -56,9 +56,10 @@ const executeQuickJsVm = ({ script: externalScript, context: externalContext, sc externalScript = removeQuotes(externalScript); } - + let managedQuickJsContext; try { - const vm = QuickJSModule.newContext(); + managedQuickJsContext = createManagedQuickJsContext(QuickJSModule); + const vm = managedQuickJsContext.vm; const { bru, req, res, ...variables } = externalContext; bru && addBruShimToContext(vm, bru); @@ -74,7 +75,7 @@ const executeQuickJsVm = ({ script: externalScript, context: externalContext, sc let scriptText = scriptType === 'template-literal' ? templateLiteralText : jsExpressionText; - const result = vm.evalCode(scriptText); + const result = vm.evalCodeRetained(scriptText); if (result.error) { let e = vm.dump(result.error); result.error.dispose(); @@ -86,6 +87,8 @@ const executeQuickJsVm = ({ script: externalScript, context: externalContext, sc } } catch (error) { console.error('Error executing the script!', error); + } finally { + managedQuickJsContext?.dispose(); } }; @@ -95,9 +98,11 @@ const executeQuickJsVmAsync = async ({ script: externalScript, context: external } externalScript = externalScript?.trim(); + let managedQuickJsContext; try { const module = await loader(); - const vm = module.newContext(); + managedQuickJsContext = createManagedQuickJsContext(module); + const vm = managedQuickJsContext.vm; // add crypto utilities required by the crypto-js library in bundledCode await addCryptoUtilsShimToContext(vm); @@ -126,17 +131,27 @@ const executeQuickJsVmAsync = async ({ script: externalScript, context: external const script = wrapScriptInClosure(externalScript, SANDBOX.QUICKJS); - const result = vm.evalCode(script, scriptPath); + const result = vm.evalCodeRetained(script, scriptPath); const promiseHandle = vm.unwrapResult(result); const resolvedResult = await vm.resolvePromise(promiseHandle); promiseHandle.dispose(); const resolvedHandle = vm.unwrapResult(resolvedResult); resolvedHandle.dispose(); - // vm.dispose(); return; } catch (error) { error.__isQuickJS = true; throw error; + } finally { + // Wait for any in-flight async work (sendRequest, axios, cookie jar, timers, + // un-awaited promises) to settle before tearing down the VM. Disposing while + // a deferred is still pending lets its later host callback touch a freed + // context, throwing `QuickJSUseAfterFree`. + try { + await managedQuickJsContext?.waitForPendingDeferreds?.(); + managedQuickJsContext?.dispose(); + } catch (teardownError) { + throw teardownError; + } } }; diff --git a/packages/bruno-js/src/sandbox/quickjs/shims/bru.js b/packages/bruno-js/src/sandbox/quickjs/shims/bru.js index 859fd0e5f..6b60ea1bc 100644 --- a/packages/bruno-js/src/sandbox/quickjs/shims/bru.js +++ b/packages/bruno-js/src/sandbox/quickjs/shims/bru.js @@ -354,6 +354,10 @@ const addBruShimToContext = (vm, bru) => { const t = vm.getString(timer); const promise = vm.newPromise(); setTimeout(() => { + // The VM may have been disposed while this native timer was pending + // (e.g. a setTimeout/sleep whose promise the script never awaited). + // Touching the VM after teardown throws QuickJSUseAfterFree, so bail out. + if (!vm.alive) return; promise.resolve(vm.newString('slept')); }, t); promise.settled.then(vm.runtime.executePendingJobs); diff --git a/packages/bruno-js/src/sandbox/quickjs/utils/index.js b/packages/bruno-js/src/sandbox/quickjs/utils/index.js index 336a1d7ae..93f2f9b15 100644 --- a/packages/bruno-js/src/sandbox/quickjs/utils/index.js +++ b/packages/bruno-js/src/sandbox/quickjs/utils/index.js @@ -1,3 +1,130 @@ +/** + * Creates a QuickJS context with centralized lifecycle management: + * - vm.evalCode() auto-disposes result handles (for shim setup code) + * - vm.evalCodeRetained() returns the raw result (for user script execution) + * - all newObject/newFunction/newArray handles are tracked and disposed on teardown + */ +const createManagedQuickJsContext = (module) => { + const vm = module.newContext(); + const disposeTracked = trackQuickJsContext(vm); + const evalCodeRetained = vm.evalCode.bind(vm); + const waitForPendingDeferreds = trackPendingDeferreds(vm); + + vm.evalCode = (code, filename = 'eval.js') => { + const result = evalCodeRetained(code, filename); + if (result.error) { + const error = vm.dump(result.error); + result.error.dispose(); + throw error; + } + result.value.dispose(); + }; + + vm.evalCodeRetained = evalCodeRetained; + + return { + vm, + waitForPendingDeferreds, + dispose: () => disposeQuickJsContext(vm, disposeTracked) + }; +}; + +/** + * Track every deferred created by the async shims (sendRequest, axios, cookie + * jar, sleep, ...) so teardown can wait for them to settle. A user script that + * fires-and-forgets async work (e.g. an un-awaited setTimeout) resolves the + * wrapping closure immediately; without this, the VM is disposed before the + * deferred's host callback runs, and touching the freed context throws + * `QuickJSUseAfterFree`. Each `.settled` resolves once the deferred is + * resolved/rejected, so awaiting them keeps the context alive long enough. + * + * The hook is installed now (at context creation) so it captures promises as + * the script runs. Returns a function that drains the captured deferreds at + * teardown; new deferreds can be created while we wait (a chained timer), so it + * drains in place until none remain. + */ + +const trackPendingDeferreds = (vm) => { + const pendingDeferreds = []; + const originalNewPromise = vm.newPromise.bind(vm); + vm.newPromise = (...args) => { + const deferred = originalNewPromise(...args); + pendingDeferreds.push(deferred.settled.catch(() => { })); + return deferred; + }; + + return async () => { + while (pendingDeferreds.length) { + const batch = pendingDeferreds.splice(0); + await Promise.all(batch); + } + }; +}; + +/** + * Tracks handles created via newObject/newFunction/newArray so they can all be + * disposed before the context. quickjs-emscripten requires every heap handle to + * be disposed individually; shims attach then drop their ref via .dispose(). + */ +const trackQuickJsContext = (vm) => { + const handles = []; + + const track = (handle) => { + handles.push(handle); + return handle; + }; + + // Replace an allocator with a wrapper that records every handle it returns, + // so teardown can dispose them all. Behaviour is otherwise identical. + const trackAllocations = (method) => { + const original = vm[method]?.bind(vm); + if (!original) { + return; + } + + vm[method] = (...args) => track(original(...args)); + }; + + ['newObject', 'newFunction', 'newArray'].forEach(trackAllocations); + + // Dispose newest-first: later handles may reference earlier ones. + return () => { + for (const handle of handles.reverse()) { + if (handle?.alive) { + handle.dispose(); + } + } + }; +}; + +/** + * Clears shim globals, drains pending QuickJS jobs, and disposes the context. + * Pass disposeTracked from trackQuickJsContext() to free shim handles first. + */ +const disposeQuickJsContext = (vm, disposeTracked) => { + if (!vm?.alive) { + return; + } + + if (typeof disposeTracked === 'function') { + disposeTracked(); + } + + // Drain the runtime's pending job queue (resolved/rejected promise callbacks) + // before disposing. Executing a job can schedule more jobs (chained `.then()`s), + // so we keep going until `hasPendingJob()` reports the queue is empty or a job + // throws. + while (vm.runtime?.hasPendingJob?.()) { + const result = vm.runtime.executePendingJobs(); + // On error, dispose the error handle and stop draining. + if (result.error) { + result.error.dispose(); + break; + } + } + vm.dispose(); +}; + const marshallToVm = (value, vm) => { if (value === undefined) { return vm.undefined; @@ -79,5 +206,8 @@ async function invokeFunction(vm, quickFn, args = []) { module.exports = { marshallToVm, - invokeFunction + invokeFunction, + createManagedQuickJsContext, + disposeQuickJsContext, + trackQuickJsContext }; diff --git a/packages/bruno-js/tests/quickjs-handle-lifecycle.spec.js b/packages/bruno-js/tests/quickjs-handle-lifecycle.spec.js new file mode 100644 index 000000000..c156f778c --- /dev/null +++ b/packages/bruno-js/tests/quickjs-handle-lifecycle.spec.js @@ -0,0 +1,307 @@ +const { describe, it, expect, beforeAll, afterAll, beforeEach } = require('@jest/globals'); +const { newQuickJSWASMModule } = require('quickjs-emscripten'); +const Bru = require('../src/bru'); +const { + createManagedQuickJsContext, + disposeQuickJsContext, + trackQuickJsContext +} = require('../src/sandbox/quickjs/utils'); + +describe('QuickJS handle lifecycle utils', () => { + let module; + + beforeAll(async () => { + module = await newQuickJSWASMModule(); + }); + + afterAll(() => { + module = null; + }); + + describe('trackQuickJsContext', () => { + it('tracks newObject, newArray, and newFunction handles', () => { + const vm = module.newContext(); + const disposeTracked = trackQuickJsContext(vm); + + const objectHandle = vm.newObject(); + const arrayHandle = vm.newArray(); + const functionHandle = vm.newFunction('trackedFn', () => vm.undefined); + + expect(objectHandle.alive).toBe(true); + expect(arrayHandle.alive).toBe(true); + expect(functionHandle.alive).toBe(true); + + disposeTracked(); + + expect(objectHandle.alive).toBe(false); + expect(arrayHandle.alive).toBe(false); + expect(functionHandle.alive).toBe(false); + expect(vm.alive).toBe(true); + + disposeQuickJsContext(vm); + }); + + it('disposes nested tracked handles without throwing', () => { + const vm = module.newContext(); + const disposeTracked = trackQuickJsContext(vm); + + const child = vm.newObject(); + const parent = vm.newObject(); + vm.setProp(parent, 'child', child); + + expect(parent.alive).toBe(true); + expect(child.alive).toBe(true); + + expect(() => disposeTracked()).not.toThrow(); + + expect(parent.alive).toBe(false); + expect(child.alive).toBe(false); + + disposeQuickJsContext(vm); + }); + + it('can be called more than once safely', () => { + const vm = module.newContext(); + const disposeTracked = trackQuickJsContext(vm); + const handle = vm.newObject(); + + disposeTracked(); + expect(handle.alive).toBe(false); + + expect(() => disposeTracked()).not.toThrow(); + expect(handle.alive).toBe(false); + + disposeQuickJsContext(vm); + }); + }); + + describe('disposeQuickJsContext', () => { + it('disposes tracked handles before disposing the context', () => { + const vm = module.newContext(); + const disposeTracked = trackQuickJsContext(vm); + const handle = vm.newArray(); + + disposeQuickJsContext(vm, disposeTracked); + + expect(handle.alive).toBe(false); + expect(vm.alive).toBe(false); + }); + + it('is a no-op when the context is already disposed', () => { + const vm = module.newContext(); + const disposeTracked = trackQuickJsContext(vm); + const handle = vm.newObject(); + + disposeQuickJsContext(vm, disposeTracked); + + expect(handle.alive).toBe(false); + expect(vm.alive).toBe(false); + expect(() => disposeQuickJsContext(vm, disposeTracked)).not.toThrow(); + }); + }); + + describe('createManagedQuickJsContext', () => { + it('clears tracked handles when dispose is called', () => { + const managed = createManagedQuickJsContext(module); + const { vm } = managed; + + const objectHandle = vm.newObject(); + const functionHandle = vm.newFunction('shimFn', () => vm.true); + + expect(objectHandle.alive).toBe(true); + expect(functionHandle.alive).toBe(true); + + managed.dispose(); + + expect(objectHandle.alive).toBe(false); + expect(functionHandle.alive).toBe(false); + expect(vm.alive).toBe(false); + }); + + it('auto-disposes evalCode results while keeping the context alive', () => { + const managed = createManagedQuickJsContext(module); + const { vm } = managed; + + expect(() => { + vm.evalCode('1 + 1'); + }).not.toThrow(); + expect(vm.alive).toBe(true); + + managed.dispose(); + expect(vm.alive).toBe(false); + }); + + it('exposes evalCodeRetained for callers that manage result handles themselves', () => { + const managed = createManagedQuickJsContext(module); + const { vm } = managed; + + const result = vm.evalCodeRetained('42'); + const valueHandle = vm.unwrapResult(result); + const value = vm.dump(valueHandle); + + expect(value).toBe(42); + expect(valueHandle.alive).toBe(true); + + valueHandle.dispose(); + expect(valueHandle.alive).toBe(false); + + managed.dispose(); + expect(vm.alive).toBe(false); + }); + }); +}); + +describe('QuickJS VM handle lifecycle', () => { + let executeQuickJsVm; + let executeQuickJsVmAsync; + let loader; + let createManagedQuickJsContextSpy; + let lastManaged; + let lastHandles; + + const makeBru = () => + new Bru({ + runtime: 'quickjs', + envVariables: {}, + runtimeVariables: {}, + processEnvVars: {}, + collectionPath: '/', + collectionName: 'Test' + }); + + const assertHandlesWereCleared = () => { + expect(lastHandles.length).toBeGreaterThan(0); + lastHandles.forEach((handle) => expect(handle.alive).toBe(false)); + expect(lastManaged.vm.alive).toBe(false); + }; + + beforeAll(async () => { + jest.resetModules(); + + const utils = require('../src/sandbox/quickjs/utils'); + const createManaged = utils.createManagedQuickJsContext; + + createManagedQuickJsContextSpy = jest + .spyOn(utils, 'createManagedQuickJsContext') + .mockImplementation((moduleArg) => { + lastHandles = []; + lastManaged = createManaged(moduleArg); + const { vm } = lastManaged; + + ['newObject', 'newArray', 'newFunction'].forEach((method) => { + const allocate = vm[method]; + vm[method] = (...args) => { + const handle = allocate(...args); + lastHandles.push(handle); + return handle; + }; + }); + + return lastManaged; + }); + + ({ executeQuickJsVm, executeQuickJsVmAsync, loader } = require('../src/sandbox/quickjs')); + await loader(); + }); + + afterAll(() => { + createManagedQuickJsContextSpy.mockRestore(); + jest.resetModules(); + }); + + beforeEach(() => { + lastManaged = null; + lastHandles = []; + createManagedQuickJsContextSpy.mockClear(); + }); + + describe('executeQuickJsVm', () => { + it('clears tracked handles when execution completes', () => { + const result = executeQuickJsVm({ + script: 'hello world', + context: { bru: makeBru() } + }); + + expect(result).toBe('hello world'); + assertHandlesWereCleared(); + }); + + it('clears tracked handles when script evaluation fails', () => { + const result = executeQuickJsVm({ + script: 'throw new Error("sync failure")', + context: { bru: makeBru() }, + scriptType: 'expression' + }); + + expect(result).toMatchObject({ message: 'sync failure' }); + assertHandlesWereCleared(); + }); + + it('does not create a managed context for literal early returns', () => { + expect(executeQuickJsVm({ script: '42', context: {} })).toBe(42); + expect(executeQuickJsVm({ script: 'true', context: {} })).toBe(true); + expect(createManagedQuickJsContextSpy).not.toHaveBeenCalled(); + }); + + it('clears tracked handles on every invocation', () => { + for (let i = 0; i < 25; i++) { + executeQuickJsVm({ + script: `\`value-${i}\``, + context: { bru: makeBru() }, + scriptType: 'expression' + }); + + assertHandlesWereCleared(); + } + expect(createManagedQuickJsContextSpy).toHaveBeenCalledTimes(25); + }); + }); + + describe('executeQuickJsVmAsync', () => { + it('clears tracked handles when async execution completes', async () => { + await executeQuickJsVmAsync({ + script: 'console.log(bru.getCollectionName());', + context: { bru: makeBru(), console: jest.fn() }, + collectionPath: '/tmp/collection' + }); + + assertHandlesWereCleared(); + }); + + it('clears tracked handles when async script execution fails', async () => { + await expect( + executeQuickJsVmAsync({ + script: 'throw new Error("async failure");', + context: { bru: makeBru() }, + collectionPath: '/tmp/collection' + }) + ).rejects.toMatchObject({ + message: 'async failure', + __isQuickJS: true + }); + + assertHandlesWereCleared(); + }); + + it('does not create a managed context for empty scripts', async () => { + await expect(executeQuickJsVmAsync({ script: '', context: {} })).resolves.toBe(''); + expect(createManagedQuickJsContextSpy).not.toHaveBeenCalled(); + }); + + it('clears tracked handles on every async invocation', async () => { + const consoleFn = jest.fn(); + + for (let i = 0; i < 10; i++) { + await executeQuickJsVmAsync({ + script: `console.log('run-${i}');`, + context: { bru: makeBru(), console: consoleFn }, + collectionPath: '/tmp/collection' + }); + + assertHandlesWereCleared(); + } + + expect(createManagedQuickJsContextSpy).toHaveBeenCalledTimes(10); + }); + }); +});