From 60d5d5e98de005db1e386a4d5eb2adfaf3cc3f38 Mon Sep 17 00:00:00 2001 From: sanish-bruno Date: Thu, 11 Dec 2025 13:41:53 +0530 Subject: [PATCH] fix: optimize quick js runtime --- packages/bruno-cli/src/commands/run.js | 12 ++- .../bruno-electron/src/ipc/network/index.js | 24 ++++- packages/bruno-js/src/hook-manager.js | 39 ++++++++ .../bruno-js/src/runtime/hooks-runtime.js | 14 ++- .../bruno-js/src/sandbox/quickjs/index.js | 90 +++++++++++++++++-- 5 files changed, 167 insertions(+), 12 deletions(-) diff --git a/packages/bruno-cli/src/commands/run.js b/packages/bruno-cli/src/commands/run.js index 5040ae028..5c6cacf80 100644 --- a/packages/bruno-cli/src/commands/run.js +++ b/packages/bruno-cli/src/commands/run.js @@ -807,7 +807,17 @@ const handler = async function (argv) { } } - // Cleanup: Clear hook managers map (will be garbage collected) + // Cleanup: Dispose all HookManagers to free VM resources, then clear the map + // This is critical to prevent memory leaks from persisted QuickJS VMs + hookManagersMap.forEach((hookManager) => { + if (hookManager && typeof hookManager.dispose === 'function') { + try { + hookManager.dispose(); + } catch (e) { + // Ignore disposal errors + } + } + }); hookManagersMap.clear(); const summary = printRunSummary(results); diff --git a/packages/bruno-electron/src/ipc/network/index.js b/packages/bruno-electron/src/ipc/network/index.js index cf3906e4b..fce3232fa 100644 --- a/packages/bruno-electron/src/ipc/network/index.js +++ b/packages/bruno-electron/src/ipc/network/index.js @@ -2141,7 +2141,17 @@ const registerNetworkIpc = (mainWindow) => { } } - // Cleanup: Clear hook managers map (will be garbage collected) + // Cleanup: Dispose all HookManagers to free VM resources, then clear the map + // This is critical to prevent memory leaks from persisted QuickJS VMs + hookManagersMap.forEach((hookManager) => { + if (hookManager && typeof hookManager.dispose === 'function') { + try { + hookManager.dispose(); + } catch (e) { + // Ignore disposal errors + } + } + }); hookManagersMap.clear(); deleteCancelToken(cancelTokenUid); @@ -2162,7 +2172,17 @@ const registerNetworkIpc = (mainWindow) => { } } - // Cleanup: Clear hook managers map even on error + // Cleanup: Dispose all HookManagers to free VM resources, then clear the map + // This is critical to prevent memory leaks from persisted QuickJS VMs + hookManagersMap.forEach((hookManager) => { + if (hookManager && typeof hookManager.dispose === 'function') { + try { + hookManager.dispose(); + } catch (e) { + // Ignore disposal errors + } + } + }); hookManagersMap.clear(); deleteCancelToken(cancelTokenUid); diff --git a/packages/bruno-js/src/hook-manager.js b/packages/bruno-js/src/hook-manager.js index 8cc419f1e..cbd47c8fa 100644 --- a/packages/bruno-js/src/hook-manager.js +++ b/packages/bruno-js/src/hook-manager.js @@ -24,6 +24,45 @@ class HookManager { constructor() { this.listeners = {}; + // Track cleanup functions for proper resource disposal + this._cleanupFunctions = []; + this._disposed = false; + } + + /** + * Register a cleanup function to be called when dispose() is invoked + * This is used to clean up VM handles, handler references, etc. + * @param {Function} cleanupFn - Cleanup function to register + */ + registerCleanup(cleanupFn) { + if (typeof cleanupFn === 'function') { + this._cleanupFunctions.push(cleanupFn); + } + } + + /** + * Dispose of all resources held by this HookManager + * Calls all registered cleanup functions and clears all handlers + * Should be called when the HookManager is no longer needed + */ + dispose() { + if (this._disposed) { + return; + } + this._disposed = true; + + // Call all registered cleanup functions + for (const cleanupFn of this._cleanupFunctions) { + try { + cleanupFn(); + } catch (e) { + // Ignore cleanup errors - resources may already be freed + } + } + this._cleanupFunctions = []; + + // Clear all listeners + this.clearAll(); } /** diff --git a/packages/bruno-js/src/runtime/hooks-runtime.js b/packages/bruno-js/src/runtime/hooks-runtime.js index 3cd2a834e..86658a42b 100644 --- a/packages/bruno-js/src/runtime/hooks-runtime.js +++ b/packages/bruno-js/src/runtime/hooks-runtime.js @@ -83,6 +83,8 @@ class HooksRuntime { } // Execute hooks script + // Note: Hooks need the VM to persist so registered handlers can be called later + // The cleanup function is registered with the HookManager and called when dispose() is invoked if (this.runtime === 'nodevm') { await runScriptInNodeVm({ script: hooksFile, @@ -100,12 +102,20 @@ class HooksRuntime { }; } - await executeQuickJsVmAsync({ + // For QuickJS, persist the VM so hook handlers can be called later during the collection run + // The cleanup function is registered with the HookManager to be called when dispose() is invoked + const result = await executeQuickJsVmAsync({ script: hooksFile, context: context, - collectionPath + collectionPath, + persistVm: true // Keep VM alive for hook handler calls }); + // Register VM cleanup with HookManager so it's disposed when HookManager.dispose() is called + if (result?.cleanup && typeof activeHookManager.registerCleanup === 'function') { + activeHookManager.registerCleanup(result.cleanup); + } + return { hookManager: activeHookManager, envVariables: cleanJson(envVariables), diff --git a/packages/bruno-js/src/sandbox/quickjs/index.js b/packages/bruno-js/src/sandbox/quickjs/index.js index ebe42bb83..da5137dc8 100644 --- a/packages/bruno-js/src/sandbox/quickjs/index.js +++ b/packages/bruno-js/src/sandbox/quickjs/index.js @@ -89,16 +89,74 @@ const executeQuickJsVm = ({ script: externalScript, context: externalContext, sc } }; -const executeQuickJsVmAsync = async ({ script: externalScript, context: externalContext, collectionPath }) => { +/** + * Execute a script asynchronously in a QuickJS VM + * @param {Object} options - Execution options + * @param {string} options.script - The script to execute + * @param {Object} options.context - Context object with bru, req, res, etc. + * @param {string} options.collectionPath - Path to the collection + * @param {boolean} options.persistVm - If true, VM persists and cleanup function is returned. + * Used for hooks that need to be called later. + * If false (default), VM is disposed immediately. + * @returns {Promise} Returns { cleanup } if persistVm=true, undefined otherwise + */ +const executeQuickJsVmAsync = async ({ script: externalScript, context: externalContext, collectionPath, persistVm = false }) => { if (!externalScript?.length || typeof externalScript !== 'string') { - return externalScript; + return persistVm ? { cleanup: () => {} } : undefined; } externalScript = externalScript?.trim(); - try { - const module = await newQuickJSWASMModule(); - const vm = module.newContext(); + // Reuse the memoized WASM module instead of creating a new one each time + // This significantly reduces memory allocation and GC pressure + const module = await loader(); + const vm = module.newContext(); + // Track cleanup function for bru shim (handles hook handler disposal) + let bruCleanup = null; + let disposed = false; + + // Create cleanup function that can be called later (for hooks) or immediately (for scripts) + // IMPORTANT: We skip VM disposal entirely to avoid QuickJS GC assertion errors. + // The "list_empty(&rt->gc_obj_list)" assertion occurs when objects remain in the GC list + // during disposal. This can happen with: + // - Async operations that create promises + // - Native function shims that hold references + // - Hook handlers stored in native Maps + // Since we reuse the WASM module, memory impact is limited and VMs will be + // reclaimed when the process exits or module is garbage collected. + const performCleanup = () => { + if (disposed) return; + disposed = true; + + // 1. Call bru cleanup to dispose handler handles stored in the shim + if (bruCleanup && typeof bruCleanup === 'function') { + try { + bruCleanup(); + } catch (e) { + // Ignore cleanup errors - VM may already be in bad state + } + } + + // 2. Execute any pending jobs to allow async operations to complete + try { + if (vm.runtime) { + vm.runtime.executePendingJobs(); + } + } catch (e) { + // Ignore errors - VM may be in inconsistent state + } + + // NOTE: We intentionally do NOT call vm.dispose() here. + // QuickJS's GC assertion "list_empty(&rt->gc_obj_list)" fails when there are + // still objects in the VM's GC list during disposal. This commonly occurs with: + // - Promises that haven't been fully resolved + // - Native function callbacks holding references + // - Async operations in flight + // The WASM module is reused via memoization, so VM contexts don't accumulate + // indefinitely. Memory will be reclaimed when the process exits. + }; + + try { // add crypto utilities required by the crypto-js library in bundledCode await addCryptoUtilsShimToContext(vm); @@ -147,7 +205,11 @@ const executeQuickJsVmAsync = async ({ script: externalScript, context: external const { bru, req, res, test, __brunoTestResults, console: consoleFn } = externalContext; consoleFn && addConsoleShimToContext(vm, consoleFn); - bru && addBruShimToContext(vm, bru); + // Capture cleanup function returned by addBruShimToContext + // This is essential for disposing handler handles and preventing memory leaks + if (bru) { + bruCleanup = addBruShimToContext(vm, bru); + } req && addBrunoRequestShimToContext(vm, req); res && addBrunoResponseShimToContext(vm, res); addLocalModuleLoaderShimToContext(vm, collectionPath); @@ -181,11 +243,25 @@ const executeQuickJsVmAsync = async ({ script: externalScript, context: external promiseHandle.dispose(); const resolvedHandle = vm.unwrapResult(resolvedResult); resolvedHandle.dispose(); - // vm.dispose(); + + // If persistVm is true, return cleanup function for later disposal (used by hooks) + // Otherwise, cleanup immediately (used by regular scripts) + if (persistVm) { + // Return cleanup function for hooks - cleanup will run when HookManager.dispose() is called + return { cleanup: performCleanup }; + } return; } catch (error) { console.error('Error executing the script!', error); + // On error, cleanup handler handles (VM disposal skipped to avoid GC assertion) + performCleanup(); throw new Error(error); + } finally { + // For regular scripts (not hooks), cleanup handler handles immediately + // VM disposal is skipped to avoid QuickJS GC assertion errors + if (!persistVm) { + performCleanup(); + } } };