Skip to content

fix: memory leaks, error handling, and retry logic improvements#11717

Closed
riftzen-bit wants to merge 2 commits intoanomalyco:devfrom
riftzen-bit:fix/additional-bugs-round2
Closed

fix: memory leaks, error handling, and retry logic improvements#11717
riftzen-bit wants to merge 2 commits intoanomalyco:devfrom
riftzen-bit:fix/additional-bugs-round2

Conversation

@riftzen-bit
Copy link
Contributor

Summary

This PR addresses several memory leaks and error handling issues discovered during a deep code review:

Bug Fixes

HIGH Priority:

  1. Timer Memory Leak in withTimeout (src/util/timeout.ts)

    • The timeout timer was only cleared on promise resolution, not rejection
    • Changed .then() to .finally() to ensure cleanup in all code paths
  2. Event Listener Memory Leak in OAuth Flow (src/mcp/index.ts)

    • Event listeners (error, exit) were not removed after one fired
    • Changed .on() to .once() for subprocess handlers

MEDIUM Priority:

  1. Silent Cache Clearing Failure (src/global/index.ts)
    • Version file was written even when cache deletion failed, corrupting cache state
    • Moved version write inside try block and added error logging

LOW Priority:

  1. Inconsistent Retry Logic Return Value (src/session/retry.ts)
    • retryable() returned raw JSON.stringify() for unrecognized errors
    • Now returns undefined for non-retryable errors (consistent behavior)
    • Updated test to match new correct behavior

Files Changed

  • packages/opencode/src/util/timeout.ts - Timer cleanup fix
  • packages/opencode/src/mcp/index.ts - Event listener fix
  • packages/opencode/src/global/index.ts - Cache error handling
  • packages/opencode/src/session/retry.ts - Retry logic fix
  • packages/opencode/test/session/retry.test.ts - Updated test

Testing

  • All 841 tests pass
  • TypeScript typecheck passes for all 12 packages

Checklist

  • Code follows project style guidelines
  • All tests pass
  • No breaking changes
  • Memory leak issues addressed

## Bug Fixes

### Timer Memory Leak in withTimeout (HIGH)
- Fixed: Timer not cleared when promise rejects
- Changed `.then()` to `.finally()` to ensure cleanup in all code paths
- Location: src/util/timeout.ts

### Event Listener Memory Leak in OAuth Flow (HIGH)
- Fixed: Event listeners not removed after one fires
- Changed `.on()` to `.once()` for subprocess error/exit handlers
- Location: src/mcp/index.ts

### Silent Cache Clearing Failure (MEDIUM)
- Fixed: Version file written even when cache deletion fails
- Moved version write inside try block, added error logging
- Location: src/global/index.ts

### Inconsistent Retry Logic Return Value (LOW)
- Fixed: retryable() returned raw JSON.stringify for unrecognized errors
- Now returns undefined for non-retryable errors (consistent behavior)
- Location: src/session/retry.ts
- Updated test to match new correct behavior

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 2, 2026 02:46
@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

The following comment was made by an LLM, it may be inaccurate:

No duplicate PRs found

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@KaranjotVendal
Copy link

@riftzen-bit does this fix work for you? This is what my machine looks like
HAKQu2CXIAA68_P

@riftzen-bit
Copy link
Contributor Author

This solution worked perfectly for me.

@KaranjotVendal
Copy link

Update on my situation I used PR #10914 and PR #11717 and Ran ./packages/opencode/script/build.ts --single, and used the resulting binary: did not help with memory issue.

@riftzen-bit riftzen-bit closed this by deleting the head repository Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments