When removing, delete from watches before removing from inotify (fixes go-fsnotify...

diff --git a/inotify.go b/inotify.go
index 3978fb54d93d1e2bb1136cde47496e74137c6f21..d7759ec8c86b020875f6ed918ffcfa37820f5a1f 100644 (file)
--- a/inotify.go
+++ b/inotify.go
@@ -132,11 +132,20 @@ func (w *Watcher) Remove(name string) error {
        if !ok {
                return fmt.Errorf("can't remove non-existent inotify watch for: %s", name)
        }
+       // inotify_rm_watch will return EINVAL if the file has been deleted;
+       // the inotify will already have been removed.
+       // That means we can safely delete it from our watches, whatever inotify_rm_watch does.
+       delete(w.watches, name)
        success, errno := syscall.InotifyRmWatch(w.fd, watch.wd)
        if success == -1 {
+               // TODO: Perhaps it's not helpful to return an error here in every case.
+               // the only two possible errors are:
+               // EBADF, which happens when w.fd is not a valid file descriptor of any kind.
+               // EINVAL, which is when fd is not an inotify descriptor or wd is not a valid watch descriptor.
+               // Watch descriptors are invalidated when they are removed explicitly or implicitly;
+               // explicitly by inotify_rm_watch, implicitly when the file they are watching is deleted.
                return errno
        }
-       delete(w.watches, name)
        return nil
 }
 

diff --git a/inotify_test.go b/inotify_test.go
index 32432c6c39925e221f93917587ebdde59fb8803c..035ee8f95d5994c82dae3513ead0124f503353de 100644 (file)
--- a/inotify_test.go
+++ b/inotify_test.go
@@ -252,3 +252,41 @@ func TestInotifyStress(t *testing.T) {
                }
        }
 }
+
+func TestInotifyRemoveTwice(t *testing.T) {
+       testDir := tempMkdir(t)
+       defer os.RemoveAll(testDir)
+       testFile := filepath.Join(testDir, "testfile")
+
+       handle, err := os.Create(testFile)
+       if err != nil {
+               t.Fatalf("Create failed: %v", err)
+       }
+       handle.Close()
+
+       w, err := NewWatcher()
+       if err != nil {
+               t.Fatalf("Failed to create watcher: %v", err)
+       }
+       defer w.Close()
+
+       err = w.Add(testFile)
+       if err != nil {
+               t.Fatalf("Failed to add testFile: %v", err)
+       }
+
+       err = os.Remove(testFile)
+       if err != nil {
+               t.Fatalf("Failed to remove testFile: %v", err)
+       }
+
+       err = w.Remove(testFile)
+       if err != syscall.EINVAL {
+               t.Fatalf("Expected EINVAL from Remove, got: %v", err)
+       }
+
+       err = w.Remove(testFile)
+       if err == syscall.EINVAL {
+               t.Fatalf("Got EINVAL again, watch was not removed")
+       }
+}