Skip to content

fix: dispose SettingsManager on session close to prevent resource leaks#454

Merged
benbrandt merged 1 commit intoagentclientprotocol:mainfrom
tbounsiar:fix/settings-manager-leak
Mar 24, 2026
Merged

fix: dispose SettingsManager on session close to prevent resource leaks#454
benbrandt merged 1 commit intoagentclientprotocol:mainfrom
tbounsiar:fix/settings-manager-leak

Conversation

@tbounsiar
Copy link
Copy Markdown
Contributor

Summary

  • SettingsManager.dispose() was never called when a session was closed via unstable_closeSession or when the Claude Agent process died unexpectedly
  • Each leaked session left up to 4 active fs.watch() file descriptors and a pending debounce timer
  • Added session.settingsManager.dispose() in both cleanup paths and updated test mocks to assert the call

Test plan

  • npm run build compiles clean
  • npm run lint passes
  • Existing session/close tests updated to verify dispose() is called
  • All 4 session/close tests pass

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 23, 2026

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Tahar BOUNSIAR.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

SettingsManager was never disposed when sessions were closed or when the
Claude Agent process died unexpectedly. Each leaked session left up to 4
active fs.watch() file descriptors and a pending debounce timer, causing
resource exhaustion on long-running instances.
@tbounsiar tbounsiar force-pushed the fix/settings-manager-leak branch from b2a1374 to 7837f2b Compare March 23, 2026 13:17
@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 23, 2026

We require contributors to sign our Contributor License Agreement, and we don't have @tbounsiar on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@tbounsiar
Copy link
Copy Markdown
Contributor Author

@cla-bot check

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 23, 2026

We require contributors to sign our Contributor License Agreement, and we don't have @tbounsiar on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 23, 2026

The cla-bot has been summoned, and re-checked this pull request!

@tbounsiar
Copy link
Copy Markdown
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Mar 23, 2026
@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 23, 2026

The cla-bot has been summoned, and re-checked this pull request!

@tbounsiar
Copy link
Copy Markdown
Contributor Author

@cla-bot check

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 23, 2026

The cla-bot has been summoned, and re-checked this pull request!

@benbrandt benbrandt enabled auto-merge (squash) March 24, 2026 21:18
@benbrandt benbrandt merged commit 5fd7873 into agentclientprotocol:main Mar 24, 2026
2 checks passed
benbrandt pushed a commit that referenced this pull request Mar 27, 2026
…485)

## Summary

- **Concurrent `setCwd()` calls** could produce duplicate file watchers
because both calls cleared `initialized` before either completed
`initialize()`. Added an `initPromise` guard so concurrent calls await
the same initialization.
- **Debounce timer callback** could fire after `dispose()`, reloading
stale settings and calling `onChange` on a disposed manager. Added a
`disposed` flag checked in the async callback before proceeding.

Follows up on #454 which fixed the missing `dispose()` calls — this PR
hardens the `SettingsManager` lifecycle against race conditions.

## Test plan

- [x] `npm run lint` passes
- [x] All settings tests pass
- [x] Verified `disposed` flag prevents stale callback execution
- [x] Verified `initPromise` deduplicates concurrent `initialize()`
calls
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.

3 participants