inotify: remove watch when renaming a watched path (#518)

Renaming a watched path is problematic; on inotify we just get a
IN_MOVED_SELF event with the old filename and that's it; no more events
for you! So if you do:

	watch one
	mv    one two
	cat   asd >two

You still continue to get events for the file "one", even though it's
now named "two" (the file descriptor doesn't care about the rename).
There is no way we can know the new event as far as I can tell,
inotifywait(1) also behaves like this. So instead of continuing in a
semi-broken state just remove the watcher, like we do for deletes.

On kqueue and FEN the situation is similar, and we actually already
removed watchers on renames.

On Windows this all works nicely; the watch is preserved and the
filename is updated. I decided to keep this as-is for now, even though
it's inconsistent. We actually fixed the Windows behaviour for the 1.6.0
release in #370 , so people do seem to care about it and use it, and
experience with the symlink change in 1.5.0 shows it's better to keep
inconsistent behaviour rather than change it. Fixing this up is
something for a v2.

Fixes #172
Fixes #503

CHANGELOG.md

@@ -13,10 +13,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changes and fixes
 
+- inotify: remove watcher if a watched path is renamed ([#518])
+
+  After a rename the reported name wasn't updated, or even an empty string.
+  Inotify doesn't provide any good facilities to update it, so just remove the
+  watcher. This is already how it worked on kqueue and FEN.
+
+  On Windows this does work, and remains working.
+
 - all: return ErrClosed on Add() when the watcher is closed ([#516])
 
 [#371]: https://github.com/fsnotify/fsnotify/pull/371
 [#516]: https://github.com/fsnotify/fsnotify/pull/516
+[#518]: https://github.com/fsnotify/fsnotify/pull/518
 
 ## [1.6.0] - 2022-10-13
 

backend_fen.go

@@ -187,11 +187,11 @@ func (w *Watcher) Close() error {
 //
 // A path can only be watched once; attempting to watch it more than once will
 // return an error. Paths that do not yet exist on the filesystem cannot be
-// added. A watch will be automatically removed if the path is deleted.
+// added.
 //
-// A path will remain watched if it gets renamed to somewhere else on the same
-// filesystem, but the monitor will get removed if the path gets deleted and
-// re-created, or if it's moved to a different filesystem.
+// A watch will be automatically removed if the watched path is deleted or
+// renamed. The exception is the Windows backend, which doesn't remove the
+// watcher on renames.
 //
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.

backend_inotify.go

@@ -120,8 +120,8 @@ type Watcher struct {
 	fd          int
 	mu          sync.Mutex // Map access
 	inotifyFile *os.File
-	watches     map[string]*watch // Map of inotify watches (key: path)
-	paths       map[int]string    // Map of watched paths (key: watch descriptor)
+	watches     map[string]*watch // Map of inotify watches (path → watch)
+	paths       map[int]string    // Map of watched paths (watch descriptor → path)
 	done        chan struct{}     // Channel for sending a "quit message" to the reader goroutine
 	doneResp    chan struct{}     // Channel to respond to Close
 }
@@ -209,11 +209,11 @@ func (w *Watcher) Close() error {
 //
 // A path can only be watched once; attempting to watch it more than once will
 // return an error. Paths that do not yet exist on the filesystem cannot be
-// added. A watch will be automatically removed if the path is deleted.
+// added.
 //
-// A path will remain watched if it gets renamed to somewhere else on the same
-// filesystem, but the monitor will get removed if the path gets deleted and
-// re-created, or if it's moved to a different filesystem.
+// A watch will be automatically removed if the watched path is deleted or
+// renamed. The exception is the Windows backend, which doesn't remove the
+// watcher on renames.
 //
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
@@ -285,25 +285,20 @@ func (w *Watcher) Remove(name string) error {
 	// Fetch the watch.
 	w.mu.Lock()
 	defer w.mu.Unlock()
-	watch, ok := w.watches[name]
 
-	// Remove it from inotify.
+	watch, ok := w.watches[name]
 	if !ok {
 		return fmt.Errorf("%w: %s", ErrNonExistentWatch, name)
 	}
 
-	// We successfully removed the watch if InotifyRmWatch doesn't return an
-	// error, we need to clean up our internal state to ensure it matches
-	// inotify's kernel state.
+	return w.remove(name, watch)
+}
+
+// Unlocked!
+func (w *Watcher) remove(name string, watch *watch) error {
 	delete(w.paths, int(watch.wd))
 	delete(w.watches, name)
 
-	// inotify_rm_watch will return EINVAL if the file has been deleted;
-	// the inotify will already have been removed.
-	// watches and pathes are deleted in ignoreLinux() implicitly and asynchronously
-	// by calling inotify_rm_watch() below. e.g. readEvents() goroutine receives IN_IGNORE
-	// so that EINVAL means that the wd is being rm_watch()ed or its file removed
-	// by another thread and we have not received IN_IGNORE event.
 	success, errno := unix.InotifyRmWatch(w.fd, watch.wd)
 	if success == -1 {
 		// TODO: Perhaps it's not helpful to return an error here in every case;
@@ -318,7 +313,6 @@ func (w *Watcher) Remove(name string) error {
 		//         are watching is deleted.
 		return errno
 	}
-
 	return nil
 }
 
@@ -417,14 +411,23 @@ func (w *Watcher) readEvents() {
 			// the "paths" map.
 			w.mu.Lock()
 			name, ok := w.paths[int(raw.Wd)]
-			// IN_DELETE_SELF occurs when the file/directory being watched is removed.
-			// This is a sign to clean up the maps, otherwise we are no longer in sync
-			// with the inotify kernel state which has already deleted the watch
-			// automatically.
+			// inotify will automatically remove the watch on deletes; just need
+			// to clean our state here.
 			if ok && mask&unix.IN_DELETE_SELF == unix.IN_DELETE_SELF {
 				delete(w.paths, int(raw.Wd))
 				delete(w.watches, name)
 			}
+			// We can't really update the state when a watched path is moved;
+			// only IN_MOVE_SELF is sent and not IN_MOVED_{FROM,TO}. So remove
+			// the watch.
+			if ok && mask&unix.IN_MOVE_SELF == unix.IN_MOVE_SELF {
+				err := w.remove(name, w.watches[name])
+				if err != nil {
+					if !w.sendError(err) {
+						return
+					}
+				}
+			}
 			w.mu.Unlock()
 
 			if nameLen > 0 {

backend_kqueue.go

@@ -241,11 +241,11 @@ func (w *Watcher) Close() error {
 //
 // A path can only be watched once; attempting to watch it more than once will
 // return an error. Paths that do not yet exist on the filesystem cannot be
-// added. A watch will be automatically removed if the path is deleted.
+// added.
 //
-// A path will remain watched if it gets renamed to somewhere else on the same
-// filesystem, but the monitor will get removed if the path gets deleted and
-// re-created, or if it's moved to a different filesystem.
+// A watch will be automatically removed if the watched path is deleted or
+// renamed. The exception is the Windows backend, which doesn't remove the
+// watcher on renames.
 //
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
@@ -676,8 +676,8 @@ func (w *Watcher) sendFileCreatedEventIfNew(filePath string, fileInfo os.FileInf
 
 func (w *Watcher) internalWatch(name string, fileInfo os.FileInfo) (string, error) {
 	if fileInfo.IsDir() {
-		// mimic Linux providing delete events for subdirectories
-		// but preserve the flags used if currently watching subdirectory
+		// mimic Linux providing delete events for subdirectories, but preserve
+		// the flags used if currently watching subdirectory
 		w.mu.Lock()
 		flags := w.dirFlags[name]
 		w.mu.Unlock()

backend_other.go

@@ -25,11 +25,11 @@ func (w *Watcher) Close() error {
 //
 // A path can only be watched once; attempting to watch it more than once will
 // return an error. Paths that do not yet exist on the filesystem cannot be
-// added. A watch will be automatically removed if the path is deleted.
+// added.
 //
-// A path will remain watched if it gets renamed to somewhere else on the same
-// filesystem, but the monitor will get removed if the path gets deleted and
-// re-created, or if it's moved to a different filesystem.
+// A watch will be automatically removed if the watched path is deleted or
+// renamed. The exception is the Windows backend, which doesn't remove the
+// watcher on renames.
 //
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.

backend_windows.go

@@ -196,11 +196,11 @@ func (w *Watcher) Close() error {
 //
 // A path can only be watched once; attempting to watch it more than once will
 // return an error. Paths that do not yet exist on the filesystem cannot be
-// added. A watch will be automatically removed if the path is deleted.
+// added.
 //
-// A path will remain watched if it gets renamed to somewhere else on the same
-// filesystem, but the monitor will get removed if the path gets deleted and
-// re-created, or if it's moved to a different filesystem.
+// A watch will be automatically removed if the watched path is deleted or
+// renamed. The exception is the Windows backend, which doesn't remove the
+// watcher on renames.
 //
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.

fsnotify_test.go

@@ -444,38 +444,21 @@ func TestWatchRename(t *testing.T) {
 		`},
 
 		{"rename watched directory", func(t *testing.T, w *Watcher, tmp string) {
-			addWatch(t, w, tmp)
-
 			dir := filepath.Join(tmp, "dir")
 			mkdir(t, dir)
 			addWatch(t, w, dir)
 
 			mv(t, dir, tmp, "dir-renamed")
 			touch(t, tmp, "dir-renamed/file")
 		}, `
-			CREATE   "/dir"           # mkdir
-			RENAME   "/dir"           # mv
-			CREATE   "/dir-renamed"
-			RENAME   "/dir"
-			CREATE   "/dir/file"      # touch
-
-			windows:
-				CREATE       "/dir"                 # mkdir
-				RENAME       "/dir"                 # mv
-				CREATE       "/dir-renamed"
-				CREATE       "/dir-renamed/file"    # touch
+			rename   /dir
 
-			# TODO: no results for the touch; this is probably a bug; windows
-			# was fixed in #370.
 			kqueue:
-				CREATE               "/dir"           # mkdir
-				CREATE               "/dir-renamed"   # mv
-				REMOVE|RENAME        "/dir"
-			fen:
-				CREATE       "/dir"                 # mkdir
-				RENAME       "/dir"                 # mv
-				CREATE       "/dir-renamed"
-				WRITE        "/dir-renamed"         # touch
+				remove|rename /dir
+
+			# TODO(v2): Windows should behave the same by default. See #518
+			windows:
+				create   /dir/file
 		`},
 
 		{"rename watched file", func(t *testing.T, w *Watcher, tmp string) {
@@ -488,19 +471,12 @@ func TestWatchRename(t *testing.T) {
 			mv(t, file, rename)
 			mv(t, rename, tmp, "rename-two")
 		}, `
-			# TODO: this should update the path. And even then, not clear what
-			# go renamed to what.
-			rename /file  # mv file rename
-			rename /file  # mv rename rename-two
-
-			# TODO: seems to lose the watch?
-			kqueue, fen:
-				rename     /file
+			rename     /file
 
-			# It's actually more correct on Windows.
+			# TODO(v2): Windows should behave the same by default. See #518
 			windows:
-				rename     /file
-				rename     /rename-one
+				rename   /file
+				rename   /rename-one
 		`},
 
 		{"re-add renamed file", func(t *testing.T, w *Watcher, tmp string) {
@@ -517,19 +493,14 @@ func TestWatchRename(t *testing.T) {
 			cat(t, "hello", file)
 		}, `
 			rename /file    # mv file rename
-			write  /rename  # cat hello >rename
+			                # Watcher gets removed on rename, so no write for /rename
 			write  /file    # cat hello >file
 
-			# TODO: wrong.
-			linux:
-			    RENAME     "/file"
-			    WRITE      "/file"
-			    WRITE      ""
-
-			# TODO: wrong.
-			kqueue, fen:
-			   RENAME      "/file"
-			   WRITE       "/file"
+			# TODO(v2): Windows should behave the same by default. See #518
+			windows:
+				rename    /file
+				write     /rename
+				write     /file
 		`},
 	}
 

mkdoc.zsh

@@ -78,11 +78,11 @@ add=$(<<EOF
 //
 // A path can only be watched once; attempting to watch it more than once will
 // return an error. Paths that do not yet exist on the filesystem cannot be
-// added. A watch will be automatically removed if the path is deleted.
+// added.
 //
-// A path will remain watched if it gets renamed to somewhere else on the same
-// filesystem, but the monitor will get removed if the path gets deleted and
-// re-created, or if it's moved to a different filesystem.
+// A watch will be automatically removed if the watched path is deleted or
+// renamed. The exception is the Windows backend, which doesn't remove the
+// watcher on renames.
 //
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.