Skip to content

Commit 669c440

Browse files
author
Vlad Losev
committed
Fixes leak of watch counters in InotifyTracker.remove.
1 parent a294104 commit 669c440

File tree

2 files changed

+56
-46
lines changed

2 files changed

+56
-46
lines changed

tail_test.go

+27
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,33 @@ func reOpen(t *testing.T, poll bool) {
377377
tailTest.Cleanup(tail, false)
378378
}
379379

380+
func TestInotify_WaitForCreateThenMove(t *testing.T) {
381+
tailTest := NewTailTest("wait-for-create-then-reopen", t)
382+
os.Remove(tailTest.path + "/test.txt") // Make sure the file does NOT exist.
383+
384+
tail := tailTest.StartTail(
385+
"test.txt",
386+
Config{Follow: true, ReOpen: true, Poll: false})
387+
388+
content := []string{"hello", "world", "endofworld"}
389+
go tailTest.VerifyTailOutput(tail, content, false)
390+
391+
time.Sleep(50 * time.Millisecond)
392+
tailTest.CreateFile("test.txt", "hello\nworld\n")
393+
time.Sleep(50 * time.Millisecond)
394+
tailTest.RenameFile("test.txt", "test.txt.rotated")
395+
time.Sleep(50 * time.Millisecond)
396+
tailTest.CreateFile("test.txt", "endofworld\n")
397+
time.Sleep(50 * time.Millisecond)
398+
tailTest.RemoveFile("test.txt.rotated")
399+
tailTest.RemoveFile("test.txt")
400+
401+
// Do not bother with stopping as it could kill the tomb during
402+
// the reading of data written above. Timings can vary based on
403+
// test environment.
404+
tailTest.Cleanup(tail, false)
405+
}
406+
380407
func reSeek(t *testing.T, poll bool) {
381408
var name string
382409
if poll {

watch/inotify_tracker.go

+29-46
Original file line numberDiff line numberDiff line change
@@ -108,28 +108,10 @@ func remove(winfo *watchInfo) error {
108108
delete(shared.done, winfo.fname)
109109
close(done)
110110
}
111-
112-
fname := winfo.fname
113-
if winfo.isCreate() {
114-
// Watch for new files to be created in the parent directory.
115-
fname = filepath.Dir(fname)
116-
}
117-
shared.watchNums[fname]--
118-
watchNum := shared.watchNums[fname]
119-
if watchNum == 0 {
120-
delete(shared.watchNums, fname)
121-
}
122111
shared.mux.Unlock()
123112

124-
// If we were the last ones to watch this file, unsubscribe from inotify.
125-
// This needs to happen after releasing the lock because fsnotify waits
126-
// synchronously for the kernel to acknowledge the removal of the watch
127-
// for this file, which causes us to deadlock if we still held the lock.
128-
if watchNum == 0 {
129-
return shared.watcher.Remove(fname)
130-
}
131113
shared.remove <- winfo
132-
return nil
114+
return <-shared.error
133115
}
134116

135117
// Events returns a channel to which FileEvents corresponding to the input filename
@@ -166,47 +148,48 @@ func (shared *InotifyTracker) addWatch(winfo *watchInfo) error {
166148
fname = filepath.Dir(fname)
167149
}
168150

151+
var err error
169152
// already in inotify watch
170-
if shared.watchNums[fname] > 0 {
171-
shared.watchNums[fname]++
172-
if winfo.isCreate() {
173-
shared.watchNums[winfo.fname]++
174-
}
175-
return nil
176-
}
177-
178-
err := shared.watcher.Add(fname)
179-
if err == nil {
180-
shared.watchNums[fname]++
181-
if winfo.isCreate() {
182-
shared.watchNums[winfo.fname]++
183-
}
153+
if shared.watchNums[fname] == 0 {
154+
err = shared.watcher.Add(fname)
184155
}
156+
shared.watchNums[fname]++
185157
return err
186158
}
187159

188160
// removeWatch calls fsnotify.RemoveWatch for the input filename and closes the
189161
// corresponding events channel.
190-
func (shared *InotifyTracker) removeWatch(winfo *watchInfo) {
162+
func (shared *InotifyTracker) removeWatch(winfo *watchInfo) error {
191163
shared.mux.Lock()
192-
defer shared.mux.Unlock()
193164

194165
ch := shared.chans[winfo.fname]
195-
if ch == nil {
196-
return
166+
if ch != nil {
167+
delete(shared.chans, winfo.fname)
168+
close(ch)
197169
}
198170

199-
delete(shared.chans, winfo.fname)
200-
close(ch)
201-
202-
if !winfo.isCreate() {
203-
return
171+
fname := winfo.fname
172+
if winfo.isCreate() {
173+
// Watch for new files to be created in the parent directory.
174+
fname = filepath.Dir(fname)
175+
}
176+
shared.watchNums[fname]--
177+
watchNum := shared.watchNums[fname]
178+
if watchNum == 0 {
179+
delete(shared.watchNums, fname)
204180
}
181+
shared.mux.Unlock()
205182

206-
shared.watchNums[winfo.fname]--
207-
if shared.watchNums[winfo.fname] == 0 {
208-
delete(shared.watchNums, winfo.fname)
183+
var err error
184+
// If we were the last ones to watch this file, unsubscribe from inotify.
185+
// This needs to happen after releasing the lock because fsnotify waits
186+
// synchronously for the kernel to acknowledge the removal of the watch
187+
// for this file, which causes us to deadlock if we still held the lock.
188+
if watchNum == 0 {
189+
err = shared.watcher.Remove(fname)
209190
}
191+
192+
return err
210193
}
211194

212195
// sendEvent sends the input event to the appropriate Tail.
@@ -241,7 +224,7 @@ func (shared *InotifyTracker) run() {
241224
shared.error <- shared.addWatch(winfo)
242225

243226
case winfo := <-shared.remove:
244-
shared.removeWatch(winfo)
227+
shared.error <- shared.removeWatch(winfo)
245228

246229
case event, open := <-shared.watcher.Events:
247230
if !open {

0 commit comments

Comments
 (0)