From 93e060272aad768b8a6ee30f0e42940c1ff9c80b Mon Sep 17 00:00:00 2001 From: Max Leiter Date: Tue, 3 Feb 2026 15:51:26 -0800 Subject: [PATCH] fix: prevent memory leaks from AbortController closures (#12024) --- packages/opencode/src/tool/codesearch.ts | 10 +- packages/opencode/src/tool/webfetch.ts | 8 +- packages/opencode/src/tool/websearch.ts | 10 +- packages/opencode/src/util/abort.ts | 35 +++++ .../opencode/test/memory/abort-leak.test.ts | 136 ++++++++++++++++++ 5 files changed, 184 insertions(+), 15 deletions(-) create mode 100644 packages/opencode/src/util/abort.ts create mode 100644 packages/opencode/test/memory/abort-leak.test.ts diff --git a/packages/opencode/src/tool/codesearch.ts b/packages/opencode/src/tool/codesearch.ts index 369cdb450..28dd4eb49 100644 --- a/packages/opencode/src/tool/codesearch.ts +++ b/packages/opencode/src/tool/codesearch.ts @@ -1,6 +1,7 @@ import z from "zod" import { Tool } from "./tool" import DESCRIPTION from "./codesearch.txt" +import { abortAfterAny } from "../util/abort" const API_CONFIG = { BASE_URL: "https://mcp.exa.ai", @@ -73,8 +74,7 @@ export const CodeSearchTool = Tool.define("codesearch", { }, } - const controller = new AbortController() - const timeoutId = setTimeout(() => controller.abort(), 30000) + const { signal, clearTimeout } = abortAfterAny(30000, ctx.abort) try { const headers: Record = { @@ -86,10 +86,10 @@ export const CodeSearchTool = Tool.define("codesearch", { method: "POST", headers, body: JSON.stringify(codeRequest), - signal: AbortSignal.any([controller.signal, ctx.abort]), + signal, }) - clearTimeout(timeoutId) + clearTimeout() if (!response.ok) { const errorText = await response.text() @@ -120,7 +120,7 @@ export const CodeSearchTool = Tool.define("codesearch", { metadata: {}, } } catch (error) { - clearTimeout(timeoutId) + clearTimeout() if (error instanceof Error && error.name === "AbortError") { throw new Error("Code search request timed out") diff --git a/packages/opencode/src/tool/webfetch.ts b/packages/opencode/src/tool/webfetch.ts index a4a54598c..c9479b9df 100644 --- a/packages/opencode/src/tool/webfetch.ts +++ b/packages/opencode/src/tool/webfetch.ts @@ -2,6 +2,7 @@ import z from "zod" import { Tool } from "./tool" import TurndownService from "turndown" import DESCRIPTION from "./webfetch.txt" +import { abortAfterAny } from "../util/abort" const MAX_RESPONSE_SIZE = 5 * 1024 * 1024 // 5MB const DEFAULT_TIMEOUT = 30 * 1000 // 30 seconds @@ -36,8 +37,7 @@ export const WebFetchTool = Tool.define("webfetch", { const timeout = Math.min((params.timeout ?? DEFAULT_TIMEOUT / 1000) * 1000, MAX_TIMEOUT) - const controller = new AbortController() - const timeoutId = setTimeout(() => controller.abort(), timeout) + const { signal, clearTimeout } = abortAfterAny(timeout, ctx.abort) // Build Accept header based on requested format with q parameters for fallbacks let acceptHeader = "*/*" @@ -55,8 +55,6 @@ export const WebFetchTool = Tool.define("webfetch", { acceptHeader = "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8" } - - const signal = AbortSignal.any([controller.signal, ctx.abort]) const headers = { "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/143.0.0.0 Safari/537.36", @@ -72,7 +70,7 @@ export const WebFetchTool = Tool.define("webfetch", { ? await fetch(params.url, { signal, headers: { ...headers, "User-Agent": "opencode" } }) : initial - clearTimeout(timeoutId) + clearTimeout() if (!response.ok) { throw new Error(`Request failed with status code: ${response.status}`) diff --git a/packages/opencode/src/tool/websearch.ts b/packages/opencode/src/tool/websearch.ts index 9cc6af72d..beedd9c7c 100644 --- a/packages/opencode/src/tool/websearch.ts +++ b/packages/opencode/src/tool/websearch.ts @@ -1,6 +1,7 @@ import z from "zod" import { Tool } from "./tool" import DESCRIPTION from "./websearch.txt" +import { abortAfterAny } from "../util/abort" const API_CONFIG = { BASE_URL: "https://mcp.exa.ai", @@ -91,8 +92,7 @@ export const WebSearchTool = Tool.define("websearch", async () => { }, } - const controller = new AbortController() - const timeoutId = setTimeout(() => controller.abort(), 25000) + const { signal, clearTimeout } = abortAfterAny(25000, ctx.abort) try { const headers: Record = { @@ -104,10 +104,10 @@ export const WebSearchTool = Tool.define("websearch", async () => { method: "POST", headers, body: JSON.stringify(searchRequest), - signal: AbortSignal.any([controller.signal, ctx.abort]), + signal, }) - clearTimeout(timeoutId) + clearTimeout() if (!response.ok) { const errorText = await response.text() @@ -137,7 +137,7 @@ export const WebSearchTool = Tool.define("websearch", async () => { metadata: {}, } } catch (error) { - clearTimeout(timeoutId) + clearTimeout() if (error instanceof Error && error.name === "AbortError") { throw new Error("Search request timed out") diff --git a/packages/opencode/src/util/abort.ts b/packages/opencode/src/util/abort.ts new file mode 100644 index 000000000..3e7cfd8b2 --- /dev/null +++ b/packages/opencode/src/util/abort.ts @@ -0,0 +1,35 @@ +/** + * Creates an AbortController that automatically aborts after a timeout. + * + * Uses bind() instead of arrow functions to avoid capturing the surrounding + * scope in closures. Arrow functions like `() => controller.abort()` capture + * request bodies and other large objects, preventing GC for the timer lifetime. + * + * @param ms Timeout in milliseconds + * @returns Object with controller, signal, and clearTimeout function + */ +export function abortAfter(ms: number) { + const controller = new AbortController() + const id = setTimeout(controller.abort.bind(controller), ms) + return { + controller, + signal: controller.signal, + clearTimeout: () => globalThis.clearTimeout(id), + } +} + +/** + * Combines multiple AbortSignals with a timeout. + * + * @param ms Timeout in milliseconds + * @param signals Additional signals to combine + * @returns Combined signal that aborts on timeout or when any input signal aborts + */ +export function abortAfterAny(ms: number, ...signals: AbortSignal[]) { + const timeout = abortAfter(ms) + const signal = AbortSignal.any([timeout.signal, ...signals]) + return { + signal, + clearTimeout: timeout.clearTimeout, + } +} diff --git a/packages/opencode/test/memory/abort-leak.test.ts b/packages/opencode/test/memory/abort-leak.test.ts new file mode 100644 index 000000000..b202c9127 --- /dev/null +++ b/packages/opencode/test/memory/abort-leak.test.ts @@ -0,0 +1,136 @@ +import { describe, test, expect } from "bun:test" +import path from "path" +import { Instance } from "../../src/project/instance" +import { WebFetchTool } from "../../src/tool/webfetch" + +const projectRoot = path.join(__dirname, "../..") + +const ctx = { + sessionID: "test", + messageID: "", + callID: "", + agent: "build", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + ask: async () => {}, +} + +const MB = 1024 * 1024 +const ITERATIONS = 50 + +const getHeapMB = () => { + Bun.gc(true) + return process.memoryUsage().heapUsed / MB +} + +describe("memory: abort controller leak", () => { + test("webfetch does not leak memory over many invocations", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const tool = await WebFetchTool.init() + + // Warm up + await tool.execute({ url: "https://example.com", format: "text" }, ctx).catch(() => {}) + + Bun.gc(true) + const baseline = getHeapMB() + + // Run many fetches + for (let i = 0; i < ITERATIONS; i++) { + await tool.execute({ url: "https://example.com", format: "text" }, ctx).catch(() => {}) + } + + Bun.gc(true) + const after = getHeapMB() + const growth = after - baseline + + console.log(`Baseline: ${baseline.toFixed(2)} MB`) + console.log(`After ${ITERATIONS} fetches: ${after.toFixed(2)} MB`) + console.log(`Growth: ${growth.toFixed(2)} MB`) + + // Memory growth should be minimal - less than 1MB per 10 requests + // With the old closure pattern, this would grow ~0.5MB per request + expect(growth).toBeLessThan(ITERATIONS / 10) + }, + }) + }, 60000) + + test("compare closure vs bind pattern directly", async () => { + const ITERATIONS = 500 + + // Test OLD pattern: arrow function closure + // Store closures in a map keyed by content to force retention + const closureMap = new Map void>() + const timers: Timer[] = [] + const controllers: AbortController[] = [] + + Bun.gc(true) + Bun.sleepSync(100) + const baseline = getHeapMB() + + for (let i = 0; i < ITERATIONS; i++) { + // Simulate large response body like webfetch would have + const content = `${i}:${"x".repeat(50 * 1024)}` // 50KB unique per iteration + const controller = new AbortController() + controllers.push(controller) + + // OLD pattern - closure captures `content` + const handler = () => { + // Actually use content so it can't be optimized away + if (content.length > 1000000000) controller.abort() + } + closureMap.set(content, handler) + const timeoutId = setTimeout(handler, 30000) + timers.push(timeoutId) + } + + Bun.gc(true) + Bun.sleepSync(100) + const after = getHeapMB() + const oldGrowth = after - baseline + + console.log(`OLD pattern (closure): ${oldGrowth.toFixed(2)} MB growth (${closureMap.size} closures)`) + + // Cleanup after measuring + timers.forEach(clearTimeout) + controllers.forEach((c) => c.abort()) + closureMap.clear() + + // Test NEW pattern: bind + Bun.gc(true) + Bun.sleepSync(100) + const baseline2 = getHeapMB() + const handlers2: (() => void)[] = [] + const timers2: Timer[] = [] + const controllers2: AbortController[] = [] + + for (let i = 0; i < ITERATIONS; i++) { + const _content = `${i}:${"x".repeat(50 * 1024)}` // 50KB - won't be captured + const controller = new AbortController() + controllers2.push(controller) + + // NEW pattern - bind doesn't capture surrounding scope + const handler = controller.abort.bind(controller) + handlers2.push(handler) + const timeoutId = setTimeout(handler, 30000) + timers2.push(timeoutId) + } + + Bun.gc(true) + Bun.sleepSync(100) + const after2 = getHeapMB() + const newGrowth = after2 - baseline2 + + // Cleanup after measuring + timers2.forEach(clearTimeout) + controllers2.forEach((c) => c.abort()) + handlers2.length = 0 + + console.log(`NEW pattern (bind): ${newGrowth.toFixed(2)} MB growth`) + console.log(`Improvement: ${(oldGrowth - newGrowth).toFixed(2)} MB saved`) + + expect(newGrowth).toBeLessThanOrEqual(oldGrowth) + }) +})