fix(listener): make file watcher setup disposal-safe - #3593
Open
GautamSharma99 wants to merge 1 commit into
Open
fix(listener): make file watcher setup disposal-safe#3593GautamSharma99 wants to merge 1 commit into
GautamSharma99 wants to merge 1 commit into
Conversation
GautamSharma99
requested review from
4shub,
carenthomas,
christinatong01,
cpacker,
jnjpng and
kl2806
as code owners
July 31, 2026 10:08
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3583.
A listener session could be disposed while detached
watch_filesetup was awaiting its dynamic filesystem imports. Because disposal only closed watchers already present in the active map, the setup could resume afterward, create a newfs.watch()handle, and retain the dead socket/session indefinitely.This change moves watcher ownership into a dedicated lifecycle object with explicit active, pending, and disposed states.
What changed
watch()creates a resource, and after the error listener is registered.stat()completion, andfile_changeddelivery against disposed or replaced watchers.unwatch_filecancellation while setup is suspended.watch_filerequests even when both arrive before asynchronous setup completes. This also prevents concurrent setup from creating and leaking two handles for the same path.createFileCommandSession()as the protocol owner while delegating watcher resource ownership tofile-watch-session.tsthrough an injectable dependency loader.Lifecycle guarantees
After
dispose()begins:watch();stat()result cannot emitfile_changed;The dependency loader is injectable only at the session boundary, which lets the race windows be tested deterministically without module-level mocks or real OS watchers.
Tests
Added deterministic coverage for:
watch()creation boundary;file_changedwhen disposal races withstat()completion;createFileCommandSession()wiring from disposal to watcher close and socket-send suppression.Validation completed:
bun test src/websocket/listener/file-watch-session.test.ts src/websocket/listener/file-commands.test.ts— 12 passed, 0 failedbun run check— all 12 checks passedStructural cleanup
Extracting watcher ownership reduced
src/websocket/listener/file-commands.tsfrom 1,030 to 949 lines. The file is now below the repository limit, so its oversized source baseline entry is removed.