-
Notifications
You must be signed in to change notification settings - Fork 73
Ignore PathNotFoundException when watching subdirs #2181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Watching subdirectories is inherently racy with file-system modifications so watcher must be prepared for `Directory.watch` to fail with `PathNotFoundException`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses a race condition in the Linux directory watcher where a PathNotFoundException
could be thrown if a subdirectory is deleted after being discovered but before a watch is established on it. The change correctly handles this by catching and ignoring the exception, which also resolves a long-standing TODO. The implementation using a StreamTransformer
is appropriate. The pull request also includes a version bump and updates to the changelog, including a fix for a typo, which are all correctly handled. I've added one suggestion to add a specific test case to ensure this fix is robust against future regressions.
PR HealthBreaking changes ✔️
This check can be disabled by tagging the PR with Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with Coverage ✔️
This check for test coverage is informational (issues shown here will not fail the PR). This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with License Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
This check can be disabled by tagging the PR with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
Watching subdirectories is inherently racy with file-system modifications so watcher must be prepared for
Directory.watch
to fail withPathNotFoundException
.This is revealed when running tests against this CL which changes timing of certain events slightly:
onDone
forFileSystemEntity.watch()
stream is now delayed by few turns of the event loop while underlying watcher is being destroyed, previously it used to fire immediately which hid away some of the inherent races in the code.