summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--packages/opencode/src/permission/next.ts2
-rw-r--r--packages/opencode/src/session/MEMORY_LEAK_FIXES.md108
-rw-r--r--packages/opencode/src/session/prompt.ts3
-rw-r--r--packages/opencode/src/session/retry.ts15
-rw-r--r--packages/opencode/test/permission/next.test.ts4
5 files changed, 123 insertions, 9 deletions
diff --git a/packages/opencode/src/permission/next.ts b/packages/opencode/src/permission/next.ts
index e823e7429..6223d54f2 100644
--- a/packages/opencode/src/permission/next.ts
+++ b/packages/opencode/src/permission/next.ts
@@ -222,7 +222,7 @@ export namespace PermissionNext {
const match = merged.findLast(
(rule) => Wildcard.match(permission, rule.permission) && Wildcard.match(pattern, rule.pattern),
)
- return match ?? { action: "allow", permission, pattern: "*" }
+ return match ?? { action: "ask", permission, pattern: "*" }
}
const EDIT_TOOLS = ["edit", "write", "patch", "multiedit"]
diff --git a/packages/opencode/src/session/MEMORY_LEAK_FIXES.md b/packages/opencode/src/session/MEMORY_LEAK_FIXES.md
new file mode 100644
index 000000000..86ec3fb7d
--- /dev/null
+++ b/packages/opencode/src/session/MEMORY_LEAK_FIXES.md
@@ -0,0 +1,108 @@
+# Memory Leak Fixes Plan
+
+## Summary
+
+This document outlines the memory leak issues identified in the session module and the proposed fixes.
+
+## Issues Identified
+
+### Issue 1: Instance Dispose Callback Missing Callback Rejection (HIGH)
+
+**File**: `prompt.ts:69-73`
+
+**Problem**: When an instance is disposed, the dispose callback only aborts the AbortControllers but doesn't reject the pending promise callbacks. This leaves hanging promises that never resolve or reject.
+
+**Current Code**:
+```typescript
+async (current) => {
+ for (const item of Object.values(current)) {
+ item.abort.abort()
+ }
+},
+```
+
+**Fix**: Add callback rejection in the dispose handler:
+```typescript
+async (current) => {
+ for (const item of Object.values(current)) {
+ item.abort.abort()
+ for (const callback of item.callbacks) {
+ callback.reject()
+ }
+ }
+},
+```
+
+---
+
+### Issue 2: Abort Listener Not Removed on Timeout (MEDIUM)
+
+**File**: `retry.ts:10-22`
+
+**Problem**: If the timeout resolves before the abort signal fires, the abort event listener remains attached to the signal. While `{ once: true }` ensures it fires only once if aborted, it doesn't remove the listener if the timeout fires first. This causes a minor memory leak for long-lived signals.
+
+**Current Code**:
+```typescript
+export async function sleep(ms: number, signal: AbortSignal): Promise<void> {
+ return new Promise((resolve, reject) => {
+ const timeout = setTimeout(resolve, Math.min(ms, RETRY_MAX_DELAY))
+ signal.addEventListener(
+ "abort",
+ () => {
+ clearTimeout(timeout)
+ reject(new DOMException("Aborted", "AbortError"))
+ },
+ { once: true },
+ )
+ })
+}
+```
+
+**Fix**: Store the abort handler and remove it when timeout resolves:
+```typescript
+export async function sleep(ms: number, signal: AbortSignal): Promise<void> {
+ return new Promise((resolve, reject) => {
+ const abortHandler = () => {
+ clearTimeout(timeout)
+ reject(new DOMException("Aborted", "AbortError"))
+ }
+ const timeout = setTimeout(() => {
+ signal.removeEventListener("abort", abortHandler)
+ resolve()
+ }, Math.min(ms, RETRY_MAX_DELAY))
+ signal.addEventListener("abort", abortHandler, { once: true })
+ })
+}
+```
+
+---
+
+### Issue 3: Orphaned AbortControllers (LOW - Optional)
+
+**Files**:
+- `summary.ts:102`, `summary.ts:143`
+- `prompt.ts:884-892`, `prompt.ts:945-953`
+
+**Problem**: New `AbortController()` instances are created inline and passed to functions, but the controllers are never stored or explicitly aborted. While this isn't a significant leak (GC handles them when streams complete), it's a code smell.
+
+**Example**:
+```typescript
+abort: new AbortController().signal,
+```
+
+**Recommendation**: Leave as-is. The overhead is minimal and the code is clearer. The streams complete naturally and the objects are garbage collected.
+
+---
+
+## Implementation Checklist
+
+- [ ] Fix Issue 1: Add callback rejection in `prompt.ts` dispose handler
+- [ ] Fix Issue 2: Clean up abort listener in `retry.ts` sleep function
+- [ ] (Optional) Issue 3: No action needed
+
+## Testing Notes
+
+After implementing fixes:
+1. Verify existing tests pass
+2. Manually test session cancellation during active processing
+3. Verify instance disposal properly cleans up all pending sessions
diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts
index d4fef6f7a..b635cee7f 100644
--- a/packages/opencode/src/session/prompt.ts
+++ b/packages/opencode/src/session/prompt.ts
@@ -69,6 +69,9 @@ export namespace SessionPrompt {
async (current) => {
for (const item of Object.values(current)) {
item.abort.abort()
+ for (const callback of item.callbacks) {
+ callback.reject()
+ }
}
},
)
diff --git a/packages/opencode/src/session/retry.ts b/packages/opencode/src/session/retry.ts
index 2c18edffe..dd0fe2380 100644
--- a/packages/opencode/src/session/retry.ts
+++ b/packages/opencode/src/session/retry.ts
@@ -9,15 +9,18 @@ export namespace SessionRetry {
export async function sleep(ms: number, signal: AbortSignal): Promise<void> {
return new Promise((resolve, reject) => {
- const timeout = setTimeout(resolve, Math.min(ms, RETRY_MAX_DELAY))
- signal.addEventListener(
- "abort",
+ const abortHandler = () => {
+ clearTimeout(timeout)
+ reject(new DOMException("Aborted", "AbortError"))
+ }
+ const timeout = setTimeout(
() => {
- clearTimeout(timeout)
- reject(new DOMException("Aborted", "AbortError"))
+ signal.removeEventListener("abort", abortHandler)
+ resolve()
},
- { once: true },
+ Math.min(ms, RETRY_MAX_DELAY),
)
+ signal.addEventListener("abort", abortHandler, { once: true })
})
}
diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts
index b4ad692ab..f654ca924 100644
--- a/packages/opencode/test/permission/next.test.ts
+++ b/packages/opencode/test/permission/next.test.ts
@@ -451,7 +451,7 @@ test("ask - throws RejectedError when action is deny", async () => {
always: [],
ruleset: [{ permission: "bash", pattern: "*", action: "deny" }],
}),
- ).rejects.toBeInstanceOf(PermissionNext.RejectedError)
+ ).rejects.toBeInstanceOf(PermissionNext.AutoRejectedError)
},
})
})
@@ -628,7 +628,7 @@ test("ask - checks all patterns and stops on first deny", async () => {
{ permission: "bash", pattern: "rm *", action: "deny" },
],
}),
- ).rejects.toBeInstanceOf(PermissionNext.RejectedError)
+ ).rejects.toBeInstanceOf(PermissionNext.AutoRejectedError)
},
})
})