From 0bc5f080e83915b4d756f9bfc2db7ea3b696fdd8 Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Mon, 29 Feb 2016 19:00:42 -0800 Subject: [PATCH 1/3] watch: Unsubscribe from fsnotify synchronously. Calling watcher.Remove() from the run() goroutine is now problematic, because with the change made in fsnotify/fsnotify#73 Remove() can now take an arbitrary amount of time, which means we can deadlock if run() is waiting for fsnotify to acknowledge the removal and fsnotify is trying to send an unrelated Event. So instead we now do part of the cleanup, including calling Remove(), synchronously, in the goroutine trying to unsubscribe. This fixes #75. Thanks to Aaron Beitch for the fix. Change-Id: I346c9eecc34b2378312b07b3c3efc41616b95380 --- watch/inotify_tracker.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/watch/inotify_tracker.go b/watch/inotify_tracker.go index f53f2c3..167cd02 100644 --- a/watch/inotify_tracker.go +++ b/watch/inotify_tracker.go @@ -107,8 +107,26 @@ func remove(winfo *watchInfo) { 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 { + shared.watcher.Remove(fname) + } shared.remove <- winfo } @@ -174,19 +192,6 @@ func (shared *InotifyTracker) removeWatch(winfo *watchInfo) { return } - fname := winfo.fname - if winfo.isCreate() { - // Watch for new files to be created in the parent directory. - fname = filepath.Dir(fname) - } - - shared.watchNums[fname]-- - if shared.watchNums[fname] == 0 { - delete(shared.watchNums, fname) - // TODO: handle error - shared.watcher.Remove(fname) - } - delete(shared.chans, winfo.fname) close(ch) From d5f9e6c0872ff6513dd258c06ee3c361e5fe2bbf Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Mon, 29 Feb 2016 22:43:14 -0800 Subject: [PATCH 2/3] tail: Fix tyop. Change-Id: I0f9b88fc327b169c748794e86efb0aa0a03b3e2a --- tail.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tail.go b/tail.go index 1bae11c..471c24d 100644 --- a/tail.go +++ b/tail.go @@ -158,10 +158,10 @@ func (tail *Tail) Stop() error { func (tail *Tail) close() { close(tail.Lines) - tail.colseFile() + tail.closeFile() } -func (tail *Tail) colseFile() { +func (tail *Tail) closeFile() { if tail.file != nil { tail.file.Close() tail.file = nil @@ -169,7 +169,7 @@ func (tail *Tail) colseFile() { } func (tail *Tail) reopen() error { - tail.colseFile() + tail.closeFile() for { var err error tail.file, err = OpenFile(tail.Filename) From 6c6f39c5865eaecfe443742ac16542f209961498 Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Mon, 29 Feb 2016 22:44:16 -0800 Subject: [PATCH 3/3] watch: Fix prototype to be more restrictive. Change-Id: Ic744312efa91cf5c2dc2810e597353a080dccb70 --- watch/inotify_tracker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/watch/inotify_tracker.go b/watch/inotify_tracker.go index 167cd02..2353ba3 100644 --- a/watch/inotify_tracker.go +++ b/watch/inotify_tracker.go @@ -133,7 +133,7 @@ func remove(winfo *watchInfo) { // Events returns a channel to which FileEvents corresponding to the input filename // will be sent. This channel will be closed when removeWatch is called on this // filename. -func Events(fname string) chan fsnotify.Event { +func Events(fname string) <-chan fsnotify.Event { shared.mux.Lock() defer shared.mux.Unlock()