Skip to content

fix(kqueue): preserve create events when child watch fails#10

Open
paralin wants to merge 1 commit intogofsnotify:mainfrom
paralin:fix/kqueue-create-write-race-gofsnotify
Open

fix(kqueue): preserve create events when child watch fails#10
paralin wants to merge 1 commit intogofsnotify:mainfrom
paralin:fix/kqueue-create-write-race-gofsnotify

Conversation

@paralin
Copy link
Copy Markdown

@paralin paralin commented May 6, 2026

This keeps directory Create notifications observable even when registering the new child watch fails, while still registering successful child watches before delivering Create so a following Write is not missed.

Tested with go test ./....

@mattn
Copy link
Copy Markdown
Member

mattn commented May 6, 2026

Could you please rebase master?

Signed-off-by: Christian Stewart <christian@aperture.us>
@paralin paralin force-pushed the fix/kqueue-create-write-race-gofsnotify branch from 44cf550 to 6fe7354 Compare May 7, 2026 03:53
@paralin
Copy link
Copy Markdown
Author

paralin commented May 7, 2026

@mattn rebased!

@mattn
Copy link
Copy Markdown
Member

mattn commented May 7, 2026

Thanks for the patch. A few issues need to be addressed before this can land:

  1. watcher_darwin.go was switched to an FSEvents-based purego backend in Replace cgo FSEvents backend with purego implementation #9, so the darwin chunk no longer applies. Please rebase and drop the darwin hunk (or move the change into the kqueue paths only). (Resolved by the latest rebase.)
  2. Could you clarify the failure mode this targets? openLocked can fail for many reasons (ENOENT race, EACCES, fd exhaustion). Emitting Create without a registered watch means the caller will never see subsequent Write/Remove for that path. Consider also surfacing the registration error via Errors.
  3. TestFileCreateWriteRace does not appear to exercise the failure path the fix targets — openLocked will succeed for a normal os.Create. Could you add a test that actually fails on main without this patch (e.g., simulate the registration failure)?
  4. Minor: please consolidate added = append(...) into a single line covering both branches, and add a brief comment explaining why we still emit Create on registration failure.

Also, the branch now conflicts with main (likely due to #12); please rebase once more.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants