From f69ef84e36f90c536a57dc040186994436a7deb8 Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Tue, 27 Oct 2015 15:45:55 -0700 Subject: [PATCH] Fix race in the detection of truncation. Before going into ChangeEvents(), the code was calling stat on the file to know where it was at, which is incorrect as stat could return the new file size post truncation. Instead we now ask the file descriptor about our current offset, so we can compare our offset to the file size to try to detect truncation. Truncation detection remains brittle, but this closes an annoying race we frequently run into. --- tail.go | 6 +++--- watch/inotify.go | 4 ++-- watch/polling.go | 9 +++++++-- watch/watch.go | 9 ++++----- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/tail.go b/tail.go index 753158d..5532ac1 100644 --- a/tail.go +++ b/tail.go @@ -290,11 +290,11 @@ func (tail *Tail) tailFileSync() { // reopened if ReOpen is true. Truncated files are always reopened. func (tail *Tail) waitForChanges() error { if tail.changes == nil { - st, err := tail.file.Stat() + pos, err := tail.file.Seek(0, os.SEEK_CUR) if err != nil { return err } - tail.changes = tail.watcher.ChangeEvents(&tail.Tomb, st) + tail.changes = tail.watcher.ChangeEvents(&tail.Tomb, pos) } select { @@ -340,7 +340,7 @@ func (tail *Tail) openReader() { } func (tail *Tail) seekEnd() error { - return tail.seekTo(SeekInfo{Offset: 0, Whence: 2}) + return tail.seekTo(SeekInfo{Offset: 0, Whence: os.SEEK_END}) } func (tail *Tail) seekTo(pos SeekInfo) error { diff --git a/watch/inotify.go b/watch/inotify.go index 258e3ef..d6635f4 100644 --- a/watch/inotify.go +++ b/watch/inotify.go @@ -58,7 +58,7 @@ func (fw *InotifyFileWatcher) BlockUntilExists(t *tomb.Tomb) error { panic("unreachable") } -func (fw *InotifyFileWatcher) ChangeEvents(t *tomb.Tomb, fi os.FileInfo) *FileChanges { +func (fw *InotifyFileWatcher) ChangeEvents(t *tomb.Tomb, pos int64) *FileChanges { changes := NewFileChanges() err := Watch(fw.Filename) @@ -66,7 +66,7 @@ func (fw *InotifyFileWatcher) ChangeEvents(t *tomb.Tomb, fi os.FileInfo) *FileCh go changes.NotifyDeleted() } - fw.Size = fi.Size() + fw.Size = pos go func() { defer RemoveWatch(fw.Filename) diff --git a/watch/polling.go b/watch/polling.go index e13e034..5479272 100644 --- a/watch/polling.go +++ b/watch/polling.go @@ -40,14 +40,19 @@ func (fw *PollingFileWatcher) BlockUntilExists(t *tomb.Tomb) error { panic("unreachable") } -func (fw *PollingFileWatcher) ChangeEvents(t *tomb.Tomb, origFi os.FileInfo) *FileChanges { +func (fw *PollingFileWatcher) ChangeEvents(t *tomb.Tomb, pos int64) *FileChanges { changes := NewFileChanges() var prevModTime time.Time // XXX: use tomb.Tomb to cleanly manage these goroutines. replace // the fatal (below) with tomb's Kill. - fw.Size = origFi.Size() + fw.Size = pos + origFi, err := os.Stat(fw.Filename) + if err != nil { + changes.NotifyDeleted() + return changes + } go func() { defer changes.Close() diff --git a/watch/watch.go b/watch/watch.go index a3c06e8..fbc08a5 100644 --- a/watch/watch.go +++ b/watch/watch.go @@ -2,10 +2,7 @@ package watch -import ( - "gopkg.in/tomb.v1" - "os" -) +import "gopkg.in/tomb.v1" // FileWatcher monitors file-level events. type FileWatcher interface { @@ -16,5 +13,7 @@ type FileWatcher interface { // deletion, renames or truncations. Returned FileChanges group of // channels will be closed, thus become unusable, after a deletion // or truncation event. - ChangeEvents(*tomb.Tomb, os.FileInfo) *FileChanges + // In order to properly report truncations, ChangeEvents requires + // the caller to pass their current offset in the file. + ChangeEvents(*tomb.Tomb, int64) *FileChanges }