From e2532248b8b41873668e8e339833f9b4083d4a71 Mon Sep 17 00:00:00 2001 From: Ken-ichirou MATSUZAWA Date: Thu, 12 Mar 2015 14:09:56 +0900 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + inotify.go | 26 ++++++++++++++++++---- inotify_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83c226a..b9e0fcb 100644 --- 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 d7759ec..06f4bba 100644 --- 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 035ee8f..4b5c4b1 100644 --- 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) } } -- 2.50.1