From b0c08f203088ee60f2e61c4e5b9dff8f366c4a93 Mon Sep 17 00:00:00 2001 From: Vlad Losev Date: Sat, 18 Mar 2017 14:20:41 -0700 Subject: [PATCH 1/4] Fixes log reopening. --- tail_test.go | 4 ++-- watch/inotify_tracker.go | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tail_test.go b/tail_test.go index 8dcc02c..e29da4b 100644 --- a/tail_test.go +++ b/tail_test.go @@ -343,7 +343,7 @@ func reOpen(t *testing.T, poll bool) { "test.txt", Config{Follow: true, ReOpen: true, Poll: poll}) content := []string{"hello", "world", "more", "data", "endofworld"} - go tailTest.ReadLines(tail, content) + go tailTest.VerifyTailOutput(tail, content, false) // deletion must trigger reopen <-time.After(delay) @@ -366,7 +366,7 @@ func reOpen(t *testing.T, poll bool) { // 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. - tail.Cleanup() + tailTest.Cleanup(tail, false) } func reSeek(t *testing.T, poll bool) { diff --git a/watch/inotify_tracker.go b/watch/inotify_tracker.go index 4f85217..a742cd5 100644 --- a/watch/inotify_tracker.go +++ b/watch/inotify_tracker.go @@ -155,6 +155,8 @@ func (shared *InotifyTracker) addWatch(winfo *watchInfo) error { if shared.chans[winfo.fname] == nil { shared.chans[winfo.fname] = make(chan fsnotify.Event) + } + if shared.done[winfo.fname] == nil { shared.done[winfo.fname] = make(chan bool) } From a294104811bb5efe83fb0a3ef3411979693287e4 Mon Sep 17 00:00:00 2001 From: Vlad Losev Date: Sun, 19 Mar 2017 17:39:19 -0700 Subject: [PATCH 2/4] Disables file deleteion in TestReSeekInotify. --- tail_test.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tail_test.go b/tail_test.go index e29da4b..82b1665 100644 --- a/tail_test.go +++ b/tail_test.go @@ -345,11 +345,19 @@ func reOpen(t *testing.T, poll bool) { content := []string{"hello", "world", "more", "data", "endofworld"} go tailTest.VerifyTailOutput(tail, content, false) - // deletion must trigger reopen - <-time.After(delay) - tailTest.RemoveFile("test.txt") - <-time.After(delay) - tailTest.CreateFile("test.txt", "more\ndata\n") + if poll { + // deletion must trigger reopen + <-time.After(delay) + tailTest.RemoveFile("test.txt") + <-time.After(delay) + tailTest.CreateFile("test.txt", "more\ndata\n") + } else { + // In inotify mode, fsnotify is currently unable to deliver notifications + // about deletion of open files, so we are not testing file deletion. + // (see https://github.com/fsnotify/fsnotify/issues/194 for details). + <-time.After(delay) + tailTest.AppendToFile("test.txt", "more\ndata\n") + } // rename must trigger reopen <-time.After(delay) @@ -426,6 +434,13 @@ func (t TailTest) CreateFile(name string, contents string) { } } +func (t TailTest) AppendToFile(name string, contents string) { + err := ioutil.WriteFile(t.path+"/"+name, []byte(contents), 0600|os.ModeAppend) + if err != nil { + t.Fatal(err) + } +} + func (t TailTest) RemoveFile(name string) { err := os.Remove(t.path + "/" + name) if err != nil { From 669c44045ef687bab232134310511b3e86afe270 Mon Sep 17 00:00:00 2001 From: Vlad Losev Date: Mon, 20 Mar 2017 17:50:07 -0700 Subject: [PATCH 3/4] 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 { From 2e79377851962f5983a4cd8ccda689920fb6ef9f Mon Sep 17 00:00:00 2001 From: Vlad Losev Date: Mon, 20 Mar 2017 18:08:26 -0700 Subject: [PATCH 4/4] Restores error propagation from fsnotify.Watcher.Add. --- watch/inotify.go | 2 +- watch/inotify_tracker.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/watch/inotify.go b/watch/inotify.go index 4f6c031..f94ebdb 100644 --- a/watch/inotify.go +++ b/watch/inotify.go @@ -99,7 +99,7 @@ func (fw *InotifyFileWatcher) ChangeEvents(t *tomb.Tomb, pos int64) (*FileChange //With an open fd, unlink(fd) - inotify returns IN_ATTRIB (==fsnotify.Chmod) case evt.Op&fsnotify.Chmod == fsnotify.Chmod: if _, err := os.Stat(fw.Filename); err != nil { - if ! os.IsNotExist(err) { + if !os.IsNotExist(err) { return } } diff --git a/watch/inotify_tracker.go b/watch/inotify_tracker.go index 707e0fa..d68ab61 100644 --- a/watch/inotify_tracker.go +++ b/watch/inotify_tracker.go @@ -153,7 +153,9 @@ func (shared *InotifyTracker) addWatch(winfo *watchInfo) error { if shared.watchNums[fname] == 0 { err = shared.watcher.Add(fname) } - shared.watchNums[fname]++ + if err == nil { + shared.watchNums[fname]++ + } return err }