kqueue: wait to close kqueue until after removing watches

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 #59

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

closes #65

diff --git a/CHANGELOG.md b/CHANGELOG.md
index bfbe734b81de95630875d026a24509f040b62d36..07d50a882b97f5bf3454ad8608c4d97055b98ec0 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 

diff --git a/integration_test.go b/integration_test.go
index ad51ab60b2212783e101e1c57332a767534954a3..59169c6afa8819ca9bd065ec2b2acbbedd07d4da 100644 (file)
--- a/integration_test.go
+++ b/integration_test.go
@@ -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":

diff --git a/kqueue.go b/kqueue.go
index 1636e74f39f60ab0217148e37ba5058406b447aa..265622d201df9850805360d064690b9237191888 100644 (file)
--- a/kqueue.go
+++ b/kqueue.go
@@ -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
 }