From 669c44045ef687bab232134310511b3e86afe270 Mon Sep 17 00:00:00 2001 From: Vlad Losev Date: Mon, 20 Mar 2017 17:50:07 -0700 Subject: [PATCH] Fixes leak of watch counters in InotifyTracker.remove. --- tail_test.go | 27 +++++++++++++++ watch/inotify_tracker.go | 75 ++++++++++++++++------------------------ 2 files changed, 56 insertions(+), 46 deletions(-) diff --git a/tail_test.go b/tail_test.go index 82b1665..38d6b84 100644 --- a/tail_test.go +++ b/tail_test.go @@ -377,6 +377,33 @@ func reOpen(t *testing.T, poll bool) { tailTest.Cleanup(tail, false) } +func TestInotify_WaitForCreateThenMove(t *testing.T) { + tailTest := NewTailTest("wait-for-create-then-reopen", t) + os.Remove(tailTest.path + "/test.txt") // Make sure the file does NOT exist. + + tail := tailTest.StartTail( + "test.txt", + Config{Follow: true, ReOpen: true, Poll: false}) + + content := []string{"hello", "world", "endofworld"} + go tailTest.VerifyTailOutput(tail, content, false) + + time.Sleep(50 * time.Millisecond) + tailTest.CreateFile("test.txt", "hello\nworld\n") + time.Sleep(50 * time.Millisecond) + tailTest.RenameFile("test.txt", "test.txt.rotated") + time.Sleep(50 * time.Millisecond) + tailTest.CreateFile("test.txt", "endofworld\n") + time.Sleep(50 * time.Millisecond) + tailTest.RemoveFile("test.txt.rotated") + tailTest.RemoveFile("test.txt") + + // Do not bother with stopping as it could kill the tomb during + // the reading of data written above. Timings can vary based on + // test environment. + tailTest.Cleanup(tail, false) +} + func reSeek(t *testing.T, poll bool) { var name string if poll { diff --git a/watch/inotify_tracker.go b/watch/inotify_tracker.go index a742cd5..707e0fa 100644 --- a/watch/inotify_tracker.go +++ b/watch/inotify_tracker.go @@ -108,28 +108,10 @@ func remove(winfo *watchInfo) error { delete(shared.done, winfo.fname) close(done) } - - fname := winfo.fname - if winfo.isCreate() { - // Watch for new files to be created in the parent directory. - fname = filepath.Dir(fname) - } - shared.watchNums[fname]-- - watchNum := shared.watchNums[fname] - if watchNum == 0 { - delete(shared.watchNums, fname) - } shared.mux.Unlock() - // If we were the last ones to watch this file, unsubscribe from inotify. - // This needs to happen after releasing the lock because fsnotify waits - // synchronously for the kernel to acknowledge the removal of the watch - // for this file, which causes us to deadlock if we still held the lock. - if watchNum == 0 { - return shared.watcher.Remove(fname) - } shared.remove <- winfo - return nil + return <-shared.error } // Events returns a channel to which FileEvents corresponding to the input filename @@ -166,47 +148,48 @@ func (shared *InotifyTracker) addWatch(winfo *watchInfo) error { fname = filepath.Dir(fname) } + var err error // already in inotify watch - if shared.watchNums[fname] > 0 { - shared.watchNums[fname]++ - if winfo.isCreate() { - shared.watchNums[winfo.fname]++ - } - return nil - } - - err := shared.watcher.Add(fname) - if err == nil { - shared.watchNums[fname]++ - if winfo.isCreate() { - shared.watchNums[winfo.fname]++ - } + if shared.watchNums[fname] == 0 { + err = shared.watcher.Add(fname) } + shared.watchNums[fname]++ return err } // removeWatch calls fsnotify.RemoveWatch for the input filename and closes the // corresponding events channel. -func (shared *InotifyTracker) removeWatch(winfo *watchInfo) { +func (shared *InotifyTracker) removeWatch(winfo *watchInfo) error { shared.mux.Lock() - defer shared.mux.Unlock() ch := shared.chans[winfo.fname] - if ch == nil { - return + if ch != nil { + delete(shared.chans, winfo.fname) + close(ch) } - delete(shared.chans, winfo.fname) - close(ch) + fname := winfo.fname + if winfo.isCreate() { + // Watch for new files to be created in the parent directory. + fname = filepath.Dir(fname) + } + shared.watchNums[fname]-- + watchNum := shared.watchNums[fname] + if watchNum == 0 { + delete(shared.watchNums, fname) + } + shared.mux.Unlock() - if !winfo.isCreate() { - return + var err error + // If we were the last ones to watch this file, unsubscribe from inotify. + // This needs to happen after releasing the lock because fsnotify waits + // synchronously for the kernel to acknowledge the removal of the watch + // for this file, which causes us to deadlock if we still held the lock. + if watchNum == 0 { + err = shared.watcher.Remove(fname) } - shared.watchNums[winfo.fname]-- - if shared.watchNums[winfo.fname] == 0 { - delete(shared.watchNums, winfo.fname) - } + return err } // sendEvent sends the input event to the appropriate Tail. @@ -241,7 +224,7 @@ func (shared *InotifyTracker) run() { shared.error <- shared.addWatch(winfo) case winfo := <-shared.remove: - shared.removeWatch(winfo) + shared.error <- shared.removeWatch(winfo) case event, open := <-shared.watcher.Events: if !open {