From 2b69a9cf3768563f7df47035be626dcfa8389f9d Mon Sep 17 00:00:00 2001 From: Martin Tournoij Date: Sun, 16 Oct 2022 21:28:33 +0200 Subject: [PATCH] kqueue: fix removing a directory Fix regression from #526, which would sometimes fail with something along the lines of: --- FAIL: TestWatchRm/remove_watched_directory (1.30s) helpers_test.go:384: fsnotify.sendDirectoryChangeEvents: open /tmp/TestWatchRmremove_watched_directory2055111636/001: no such file or directory fsnotify_test.go:750: have: REMOVE "/a" REMOVE "/b" REMOVE "/c" REMOVE "/d" REMOVE "/e" REMOVE "/f" REMOVE "/g" want: REMOVE "/" REMOVE "/a" REMOVE "/b" REMOVE "/c" REMOVE "/d" REMOVE "/e" REMOVE "/f" REMOVE "/g" REMOVE "/h" REMOVE "/i" REMOVE "/j" We can just always ignore a directory no longer existing; kqueue should still send the correct events. Also noticed the error on sendFileCreatedEventIfNew() wasn't really being sent properly, so deal with that as well. --- CHANGELOG.md | 3 ++- backend_kqueue.go | 44 +++++++++++++++++++++++++++++--------------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68199895..3b7f9e1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ Unreleased - all: return `ErrClosed` on `Add()` when the watcher is closed ([#516]) -- kqueue: deal with `rm -rf watched-dir` better ([#526]) +- kqueue: deal with `rm -rf watched-dir` better ([#526], [#537]) - Add `Watcher.Errors` and `Watcher.Events` to the no-op `Watcher` in `backend_other.go`, making it easier to use on unsupported platforms such as @@ -53,6 +53,7 @@ Unreleased [#525]: https://github.com/fsnotify/fsnotify/pull/525 [#526]: https://github.com/fsnotify/fsnotify/pull/526 [#528]: https://github.com/fsnotify/fsnotify/pull/528 +[#537]: https://github.com/fsnotify/fsnotify/pull/537 1.6.0 - 2022-10-13 ------------------- diff --git a/backend_kqueue.go b/backend_kqueue.go index f9a0951f..b6f5121a 100644 --- a/backend_kqueue.go +++ b/backend_kqueue.go @@ -194,8 +194,8 @@ func (w *Watcher) sendEvent(e Event) bool { case w.Events <- e: return true case <-w.done: + return false } - return false } // Returns true if the error was sent, or false if watcher is closed. @@ -204,8 +204,8 @@ func (w *Watcher) sendError(err error) bool { case w.Errors <- err: return true case <-w.done: + return false } - return false } // Close removes all watches and closes the events channel. @@ -544,7 +544,7 @@ func (w *Watcher) readEvents() { } if path.isDir && event.Has(Write) && !event.Has(Remove) { - w.sendDirectoryChangeEvents(event.Name, false) + w.sendDirectoryChangeEvents(event.Name) } else { if !w.sendEvent(event) { closed = true @@ -553,20 +553,30 @@ func (w *Watcher) readEvents() { } if event.Has(Remove) { - // Look for a file that may have overwritten this. - // For example, mv f1 f2 will delete f2, then create f2. + // Look for a file that may have overwritten this; for example, + // mv f1 f2 will delete f2, then create f2. if path.isDir { fileDir := filepath.Clean(event.Name) w.mu.Lock() _, found := w.watches[fileDir] w.mu.Unlock() if found { - w.sendDirectoryChangeEvents(fileDir, true) + err := w.sendDirectoryChangeEvents(fileDir) + if err != nil { + if !w.sendError(err) { + closed = true + } + } } } else { filePath := filepath.Clean(event.Name) if fileInfo, err := os.Lstat(filePath); err == nil { - w.sendFileCreatedEventIfNew(filePath, fileInfo) + err := w.sendFileCreatedEventIfNew(filePath, fileInfo) + if err != nil { + if !w.sendError(err) { + closed = true + } + } } } } @@ -634,24 +644,28 @@ func (w *Watcher) watchDirectoryFiles(dirPath string) error { // // This functionality is to have the BSD watcher match the inotify, which sends // a create event for files created in a watched directory. -func (w *Watcher) sendDirectoryChangeEvents(dir string, ignoreNotExists bool) { +func (w *Watcher) sendDirectoryChangeEvents(dir string) error { files, err := ioutil.ReadDir(dir) if err != nil { - // Directory could have been deleted already; just ignore that. - if ignoreNotExists && errors.Is(err, os.ErrNotExist) { - return - } - if !w.sendError(fmt.Errorf("fsnotify.sendDirectoryChangeEvents: %w", err)) { - return + // Directory no longer exists: we can ignore this safely. kqueue will + // still give us the correct events. + if errors.Is(err, os.ErrNotExist) { + return nil } + return fmt.Errorf("fsnotify.sendDirectoryChangeEvents: %w", err) } for _, fi := range files { err := w.sendFileCreatedEventIfNew(filepath.Join(dir, fi.Name()), fi) if err != nil { - return + // Don't need to send an error if this file isn't readable. + if errors.Is(err, unix.EACCES) || errors.Is(err, unix.EPERM) { + return nil + } + return fmt.Errorf("fsnotify.sendDirectoryChangeEvents: %w", err) } } + return nil } // sendFileCreatedEvent sends a create event if the file isn't already being tracked.