Skip to content

Commit

Permalink
kqueue: wait to close kqueue until after removing watches
Browse files Browse the repository at this point in the history
sending done would close w.kq before Remove had a chance to remove the watches with EV_DELETE, resulting in a file handle leak.

ref howeyc#59

also make Close() report the first error returned by Remove and continue.

closes howeyc#65
  • Loading branch information
nathany committed Feb 7, 2015
1 parent 82a2c3d commit a1aa865
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## master / 2015-02-07

* inotify: closing watcher should now always shut down goroutine [#63](https://github.com/go-fsnotify/fsnotify/pull/63) (thanks @PieterD)
* kqueue: close kqueue after removing watches, fixes [#59](https://github.com/go-fsnotify/fsnotify/issues/59)

## v1.1.1 / 2015-02-05

Expand Down
15 changes: 15 additions & 0 deletions integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,21 @@ func TestConcurrentRemovalOfWatch(t *testing.T) {
<-removed2
}

func TestClose(t *testing.T) {
// Regression test for #59 bad file descriptor from Close
testDir := tempMkdir(t)
defer os.RemoveAll(testDir)

watcher := newWatcher(t)
if err := watcher.Add(testDir); err != nil {
t.Fatalf("Expected no error on Add, got %v", err)
}
err := watcher.Close()
if err != nil {
t.Fatalf("Expected no error on Close, got %v.", err)
}
}

func testRename(file1, file2 string) error {
switch runtime.GOOS {
case "windows", "plan9":
Expand Down
12 changes: 8 additions & 4 deletions kqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,20 @@ func (w *Watcher) Close() error {
w.isClosed = true
w.mu.Unlock()

// Send "quit" message to the reader goroutine:
w.done <- true

w.mu.Lock()
ws := w.watches
w.mu.Unlock()

var err error
for name := range ws {
w.Remove(name)
if e := w.Remove(name); e != nil && err == nil {
err = e
}
}

// Send "quit" message to the reader goroutine:
w.done <- true

return nil
}

Expand Down

0 comments on commit a1aa865

Please sign in to comment.