Skip to content

Fix kqueue descriptor ownership across watch replacement - #65495

Open
Jake Bailey (jakebailey) wants to merge 1 commit into
microsoft:mainfrom
jakebailey:compiler-fixes/kqueue-descriptor-lifetime
Open

Jake Bailey (jakebailey) wants to merge 1 commit into
microsoft:mainfrom
jakebailey:compiler-fixes/kqueue-descriptor-lifetime

Conversation

@jakebailey

Copy link
Copy Markdown
Member

Subscriptions can retain descriptor numbers after entries are reopened. Closing a stale number can close an unrelated file and leave the current watch descriptor open.

Fix this.

Caught in some automated code review.

Copilot AI balanced review requested due to automatic review settings November 14, 2026 00:20
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Nov 14, 2026
@typescript-automation typescript-automation Bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Nov 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Ancestor renames or removals leave nested watches and descriptors active, allowing later writes to emit updates for nonexistent paths instead of terminating the affected watches.

1 open issue
What changed in this PR

This PR fixes descriptor ownership in the native file watcher's kqueue backend so replacing a watched entry does not cause later cleanup to close an unrelated file descriptor.

Changes:

  • Makes each entry own its current descriptor instead of caching descriptor numbers in subscriptions.
  • Centralizes descriptor cleanup and holds the backend lock throughout directory comparison.
  • Adds regression tests for file replacement, root deletion or rename, overlapping watches, and shutdown.
File Description
tsc/​internal/​fswatch/​kqueue.go Updates descriptor ownership, subscription cleanup, and locking.
tsc/​internal/​fswatch/​kqueue_test.go Adds descriptor-lifetime tests for replacement and watch cleanup.

Review effort: Balanced

Comment thread tsc/internal/fswatch/kqueue.go Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Directory invalidation can incorrectly terminate watches attached to replacement directories and leave unsubscribed entries stale, causing missed delete events.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment thread tsc/internal/fswatch/kqueue.go Resolved
Comment thread tsc/internal/fswatch/kqueue.go Resolved
Subscriptions can retain stale descriptor numbers after replacement.
Closing them can close unrelated files and leak current descriptors.
Path-only invalidation also loses failed-open entries and can terminate
watches already attached to replacement inodes.

Keep entry ownership separate from kernel subscriptions and record file
identity when opening descriptors. Clean only affected old trees,
including failed-open entries, while preserving replacement watches and
unrelated overlaps. Cover ancestor loss, replacement and cleanup.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical identity-check mismatch between Fstat and Lstat can incorrectly terminate an unchanged directory symlink watch when an unrelated sibling is created under a watched parent.

1 open issue
2 resolved since last review

Review effort: Balanced

@@ -636,14 +581,9 @@ func (b *kqueueBackend) compareDir(_ int, path string, touched map[*dirWatch]str
if unix.Fstat(fd, &fdSt) == nil && unix.Lstat(fullWatchPath, &pathSt) == nil {

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

2 participants