Skip to content

Commit

Permalink
kqueue: fix removing a directory
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
arp242 committed Nov 16, 2022
1 parent 7ed195f commit 2b69a9c
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 16 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -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
Expand All @@ -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
-------------------
Expand Down
44 changes: 29 additions & 15 deletions backend_kqueue.go
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
}
}
}
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 2b69a9c

Please sign in to comment.