inotify: sync Watcher with underlying inotify with IN_IGNORE

Watcher.watches and paths are not deleted when entry was removed automatically. Deleting those entry at ignoreLinux() to fix it.

This changes removing invalid watch name, it (should) always return "can't remove non-existent..."

closes #73

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 83c226af5006d3e7b49eabde64bb8db9527f5328..b9e0fcb990b92c97343802219aa54ad9e3e03f00 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## master / 2015-10-17
 
 * inotify: use epoll_create1 for arm64 support (requires Linux 2.6.27 or later) [#100](https://github.com/go-fsnotify/fsnotify/pull/100) (thanks @suihkulokki)
+* inotify: fix path leaks [#73](https://github.com/go-fsnotify/fsnotify/pull/73) (thanks @chamaken)
 
 ## v1.2.1 / 2015-10-14
 

diff --git a/inotify.go b/inotify.go
index d7759ec8c86b020875f6ed918ffcfa37820f5a1f..06f4bba88e08658a1261132622b41d240b113487 100644 (file)
--- a/inotify.go
+++ b/inotify.go
@@ -23,6 +23,7 @@ type Watcher struct {
        Events   chan Event
        Errors   chan error
        mu       sync.Mutex // Map access
+       cv       *sync.Cond // sync removing on rm_watch with IN_IGNORE
        fd       int
        poller   *fdPoller
        watches  map[string]*watch // Map of inotify watches (key: path)
@@ -54,6 +55,7 @@ func NewWatcher() (*Watcher, error) {
                done:     make(chan struct{}),
                doneResp: make(chan struct{}),
        }
+       w.cv = sync.NewCond(&w.mu)
 
        go w.readEvents()
        return w, nil
@@ -134,8 +136,10 @@ func (w *Watcher) Remove(name string) error {
        }
        // 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)
+       // 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 := syscall.InotifyRmWatch(w.fd, watch.wd)
        if success == -1 {
                // TODO: Perhaps it's not helpful to return an error here in every case.
@@ -146,6 +150,14 @@ func (w *Watcher) Remove(name string) error {
                // explicitly by inotify_rm_watch, implicitly when the file they are watching is deleted.
                return errno
        }
+
+       // wait until ignoreLinux() deleting maps
+       exists := true
+       for exists {
+               w.cv.Wait()
+               _, exists = w.watches[name]
+       }
+
        return nil
 }
 
@@ -249,7 +261,7 @@ func (w *Watcher) readEvents() {
                        event := newEvent(name, mask)
 
                        // Send the events that are not ignored on the events channel
-                       if !event.ignoreLinux(mask) {
+                       if !event.ignoreLinux(w, raw.Wd, mask) {
                                select {
                                case w.Events <- event:
                                case <-w.done:
@@ -266,9 +278,15 @@ func (w *Watcher) readEvents() {
 // Certain types of events can be "ignored" and not sent over the Events
 // channel. Such as events marked ignore by the kernel, or MODIFY events
 // against files that do not exist.
-func (e *Event) ignoreLinux(mask uint32) bool {
+func (e *Event) ignoreLinux(w *Watcher, wd int32, mask uint32) bool {
        // Ignore anything the inotify API says to ignore
        if mask&syscall.IN_IGNORED == syscall.IN_IGNORED {
+               w.mu.Lock()
+               defer w.mu.Unlock()
+               name := w.paths[int(wd)]
+               delete(w.paths, int(wd))
+               delete(w.watches, name)
+               w.cv.Broadcast()
                return true
        }
 

diff --git a/inotify_test.go b/inotify_test.go
index 035ee8f95d5994c82dae3513ead0124f503353de..4b5c4b18492a0cdfe41debaf58862d85ee3a671e 100644 (file)
--- a/inotify_test.go
+++ b/inotify_test.go
@@ -7,6 +7,7 @@
 package fsnotify
 
 import (
+       "fmt"
        "os"
        "path/filepath"
        "syscall"
@@ -281,12 +282,60 @@ func TestInotifyRemoveTwice(t *testing.T) {
        }
 
        err = w.Remove(testFile)
-       if err != syscall.EINVAL {
-               t.Fatalf("Expected EINVAL from Remove, got: %v", err)
+       if err == nil {
+               t.Fatalf("no error on removing invalid file")
        }
+       s1 := fmt.Sprintf("%s", err)
 
        err = w.Remove(testFile)
-       if err == syscall.EINVAL {
-               t.Fatalf("Got EINVAL again, watch was not removed")
+       if err == nil {
+               t.Fatalf("no error on removing invalid file")
+       }
+       s2 := fmt.Sprintf("%s", err)
+
+       if s1 != s2 {
+               t.Fatalf("receive different error - %s / %s", s1, s2)
+       }
+}
+
+func TestInotifyInnerMapLength(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)
+       }
+       go func() {
+               for err := range w.Errors {
+                       t.Fatalf("error received: %s", err)
+               }
+       }()
+
+       err = os.Remove(testFile)
+       if err != nil {
+               t.Fatalf("Failed to remove testFile: %v", err)
+       }
+       _ = <-w.Events                      // consume Remove event
+       <-time.After(50 * time.Millisecond) // wait IN_IGNORE propagated
+
+       if len(w.watches) != 0 {
+               t.Fatalf("Expected watches len is 0, but got: %d, %v", len(w.watches), w.watches)
+       }
+       if len(w.paths) != 0 {
+               t.Fatalf("Expected paths len is 0, but got: %d, %v", len(w.paths), w.paths)
        }
 }