File watching: a native watcher leaks when the last listener goes away during an ignore refresh #9460

Activity

1. github-actions commented on Nov 7, 2026

   

   github-actionsBot

   on Nov 7, 2026 – with GitHub Actions

   Contributor

   More actions

   Summary

   The report holds up when the code is read. FsWatcherManager.refreshIgnores drops the old native subscription and then calls attachNativeSubscription(state) again. Neither function checks, after backend.subscribe resolves, whether the state was disposed while the call was pending. If the last listener releases, or manager.close() runs, inside that window, disposeWatcherState has already removed the state from watchers, aborted it and set state.subscription = null. The new subscription is then written onto that dead state, and nothing will unsubscribe it again. The parcel/FSEvents/inotify watcher leaks until the process exits. If the dispose happens earlier in the re-attach (for example while the ignore providers or normalizePath are running), the attach throws an abort error. refreshIgnores then logs a false "ignore refresh re-attach failed — entering recovery" and starts a recovery interval on the disposed state.

   Affected code

   • packages/workspace-fs/src/watch.ts:632-668: attachNativeSubscription. The only abort check is at :652, before the await this.backend.subscribe(...) at :668. The resolved subscription is assigned to state.subscription without checking again.
   • packages/workspace-fs/src/watch.ts:835-898: refreshIgnores. The ownership check at :868-873 runs before the re-attach. After await this.attachNativeSubscription(state) at :875 returns, nothing checks it again. The catch at :876-894 treats an abort the same as a real failure: it calls console.error and setInterval(tryRecover).
   • packages/workspace-fs/src/watch.ts:375-390 (release) and :405-413 (close): both call disposeWatcherState (:415-432), which only unsubscribes the subscription that exists at that moment. That is null during the swap gap.
   • For comparison, tryRecover (:1171-1179) already handles this race: it unsubscribes the orphaned subscription when this.watchers.get(state.absolutePath) !== state. In createWatcher, the .then at :352-355 also disposes a state created after an abort. refreshIgnores is the only caller with no guard.
   • Caller: packages/host-service/src/events/git-watcher.ts:455-460 → packages/host-service/src/runtime/filesystem/filesystem.ts:101-102.

   Suspected root cause

   There is a check-then-act gap across the await at watch.ts:668. refreshIgnores checks ownership at :868, then waits for backend.subscribe. A concurrent release()/close() disposes the state and finds subscription === null. The pending subscribe then resolves and assigns a live subscription to the disposed state. No later code path can reach it, because the state is no longer in this.watchers.

   The secondary symptom is smaller. A recovery interval on the dead state is cleared on the first tryRecover tick (:1141-1146), so it does not leak. The error log is still wrong, though.

   Reproduction sketch (bun:test)

   FsWatcherManagerOptions.backend can be injected (watch.ts:264, :327), so this can be tested without real timing:

   1. Write a fake NativeWatchBackend. Its subscribe returns a deferred promise you control, and each subscription it returns counts its unsubscribe calls.
   2. Create new FsWatcherManager({ backend: fake, listGitIgnoredDirs: async () => ignored }) with ignored = ["buildout"] on a temp root. Then const release = await manager.subscribe({ absolutePath: root }, () => {}). Resolve the first subscribe right away.
   3. Set ignored = [] and call const p = manager.refreshIgnores(root). Wait until the fake records the second subscribe call and leave it pending.
   4. await release() (or await manager.close()), then resolve the pending subscribe and await p.
   5. Assert that the second subscription was unsubscribed and that p resolved to false. On current main, the first assertion should fail.

   A variant: call release() before step 3's providers resolve (make listGitIgnoredDirs deferred too). Assert that console.error was not called with the "re-attach failed" message.

   watch-gitignored-prune.test.ts:239 (FsWatcherManager.refreshIgnores) is the natural home for this test. It must not wait on real timers, per the repo test rules.

   Fix direction

   The approach in the issue fits the existing code:

   • In attachNativeSubscription, after await this.backend.subscribe(...), check state.controller.signal.aborted. If it is set, await unsubscribeQuietly(subscription) and throw the abort reason. Only assign state.subscription if the check passes. This covers all three callers (createWatcher, refreshIgnores, tryRecover). It makes the after-the-fact cleanup in tryRecover (:1174-1179) a fallback, though keeping that cleanup is harmless.
   • In the refreshIgnores catch, return false without logging or starting recovery when state.controller.signal.aborted is set, or when this.watchers.get(state.absolutePath) !== state.

   Note: this is based only on reading the code. I did not run anything.