Merge pull request #537 from fsnotify/fix-526

kqueue: fix removing a directory

.cirrus.yml

@@ -0,0 +1,12 @@
+freebsd_task:
+  name: 'FreeBSD'
+  freebsd_instance:
+    image_family: freebsd-13-1
+  install_script:
+    - pkg update -f
+    - pkg install -y go
+  test_script:
+      # run tests as user "cirrus" instead of root
+    - pw useradd cirrus -m
+    - chown -R cirrus:cirrus .
+    - sudo -u cirrus go test -race ./...

.github/workflows/test.yml

@@ -72,22 +72,6 @@ jobs:
         run: |
           go test -race ./...
 
-  # FreeBSD
-  testFreeBSD:
-    runs-on: macos-12
-    name: test (freebsd, 1.18)
-    steps:
-      - uses: actions/checkout@v3
-      - name: test (freebsd, 1.18)
-        id: test
-        uses: vmactions/freebsd-vm@v0.2.0
-        with:
-          usesh: true
-          prepare: pkg install -y go
-          run: |
-            pw user add -n action -m
-            su action -c 'go test -race ./...'
-
   # OpenBSD; no -race as the VM doesn't include the comp set.
   #
   # TODO: should probably add this, but on my local machine the tests time out
@@ -102,7 +86,7 @@ jobs:
       - uses: actions/checkout@v3
       - name: test (openbsd, 1.17)
         id: test
-        uses: vmactions/openbsd-vm@v0.0.6
+        uses: vmactions/openbsd-vm@v0
         with:
           prepare: pkg_add go
           run: |
@@ -117,7 +101,7 @@ jobs:
       - uses: actions/checkout@v3
       - name: test (netbsd, 1.18)
         id: test
-        uses: vmactions/netbsd-vm@v0.0.4
+        uses: vmactions/netbsd-vm@v0
         with:
           prepare: pkg_add go
           # TODO: no -race for the same reason as OpenBSD (the timing; it does run).

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
 -------------------

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.