FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(memory): Fix event listener leaks in TUI and Slack bot by sauerdaniel · Pull Request #9147 · anomalyco/opencode · GitHub

/ opencode Public

fix(memory): Fix event listener leaks in TUI and Slack bot - #9147

Closed
sauerdaniel wants to merge 1 commit into
anomalyco:devfrom
sauerdaniel:pr/memory-tui
ConversationCommits (1)ChecksFiles changed
Closed

fix(memory): Fix event listener leaks in TUI and Slack bot#9147
sauerdaniel wants to merge 1 commit into
anomalyco:devfrom
sauerdaniel:pr/memory-tui

Conversation

sauerdaniel commented Jan 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fix memory leaks from event listeners not being properly cleaned up in TUI components and the Slack bot.

Fixes #9155

Problems

1. TUI Sync Context (sync.tsx)

The sdk.event.listen() call sets up event listeners but the returned unsubscribe function is never called.

2. Tooltip Component (tooltip.tsx)

Mouse event listeners are added to elements but never removed on component unmount.

3. Slack Bot (slack/index.ts)

  • Sessions Map grows without bounds
  • No cleanup of old/stale sessions
  • No graceful shutdown handling

Solution

TUI Sync Context

Store and call unsubscribe function in onCleanup:

const unsubscribe = sdk.event.listen((e) => { ... })
onCleanup(unsubscribe)

Tooltip Component

Add proper cleanup in onCleanup handler.

Slack Bot

  • Add lastUsed timestamp to sessions
  • Add periodic cleanup of sessions older than 1 hour
  • Add MAX_SESSIONS limit (100)
  • Add graceful shutdown handlers for SIGINT/SIGTERM

Changes

  • packages/opencode/src/cli/cmd/tui/context/sync.tsx - Store and call unsubscribe
  • packages/ui/src/components/tooltip.tsx - Add event listener cleanup
  • packages/slack/src/index.ts - Add session management and shutdown handlers

Testing

  • TypeScript compilation passes (bun turbo typecheck)
  • Unit tests pass (725 tests, 0 failures)

Note: Manual TUI and Slack bot testing was not performed. Memory leak verification requires runtime memory profiling.

github-actions Bot commented Jan 17, 2026

Copy link
Copy Markdown
Contributor

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 Bot commented Jan 17, 2026

Copy link
Copy Markdown
Contributor

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

Potential Duplicate PRs Found

  1. fix(tui): clean up SDK event listeners on unmount #8254 - fix(tui): clean up SDK event listeners on unmount

  2. fix(core): add dispose functions to prevent subscription memory leaks #7032 - fix(core): add dispose functions to prevent subscription memory leaks

These PRs appear to target the same memory leak issues, particularly around TUI event listener cleanup. Review their implementation status to ensure no duplicated work.

github-actions Bot mentioned this pull request Jan 17, 2026
Closed
2 tasks
github-actions Bot mentioned this pull request Jan 24, 2026
Closed
thdxr force-pushed the dev branch 3 times, most recently from f1ae801 to 08fa7f7 Compare January 30, 2026 14:37

sauerdaniel commented Feb 10, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto latest dev and resolved merge conflicts.\n\nChanges:\n- tooltip.tsx: Added imports (children, onCleanup, onMount) and event listener cleanup logic\n- sync.tsx: Auto-merged successfully\n- slack/index.ts: Auto-merged successfully\n\nAll typechecks pass. Ready for review.

This was referenced Feb 11, 2026
Closed
Closed
sauerdaniel force-pushed the pr/memory-tui branch 6 times, most recently from f4fd64a to fab3aed Compare February 16, 2026 11:06

sauerdaniel commented Feb 16, 2026

Copy link
Copy Markdown
Contributor Author

Status update: implementation is complete and linked issue coverage remains intact (Fixes #9155).

Current blocker is CI only:

  • e2e (windows) fails with repeated NotFoundError
  • test (linux) then fails on the upstream gate because e2e is red

No new code changes pending on this PR right now.

sauerdaniel mentioned this pull request Feb 16, 2026
Closed
fix(memory): Fix event listener memory leaks in TUI and Slack bot
40a921c 
Clean up SDK event listeners on unmount in TUI sync context and
improve Slack bot session management to prevent leaks.

TUI changes (sync.tsx):
- Store unsubscribe function from sdk.event.listen()
- Call unsubscribe in onCleanup handler

Slack changes:
- Add lastUsed timestamp to sessions
- Add periodic cleanup of old sessions (1 hour timeout)
- Add graceful shutdown handlers for SIGINT/SIGTERM
- Limit max sessions to prevent unbounded growth
This was referenced Feb 22, 2026
Closed
Closed
binarydoubling mentioned this pull request Mar 9, 2026
Closed

github-actions Bot commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Closing this pull request because it has had no updates for more than 60 days. If you plan to continue working on it, feel free to reopen or open a new PR.

github-actions Bot closed this Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory leak: TUI and Slack bot event listeners not cleaned up

1 participant

Back | FazBrowse Home | New Git URL