From: Pawel Knap <pawelknap88@gmail.com>
Date: Wed, 25 Nov 2015 23:22:45 +0000 (+0100)
Subject: kqueue: fix race condition in Close
X-Git-Tag: v1.7.2~208
X-Git-Url: https://go.fuhry.dev/?a=commitdiff_plain;h=a005bab8d417b7225068819cddfa9e8c0629f283;p=fsnotify.git

kqueue: fix race condition in Close

fixes #99
closes #105

thanks to @djui for reporting the issue and @ppknap for writing a test.

Add test for concurrent file removes and watcher.Close calls
---

diff --git a/integration_test.go b/integration_test.go
index 49e48ff..8b7e9d3 100644
--- a/integration_test.go
+++ b/integration_test.go
@@ -44,6 +44,16 @@ func tempMkdir(t *testing.T) string {
 	return dir
 }
 
+// tempMkFile makes a temporary file.
+func tempMkFile(t *testing.T, dir string) string {
+	f, err := ioutil.TempFile(dir, "fsnotify")
+	if err != nil {
+		t.Fatalf("failed to create test file: %v", err)
+	}
+	defer f.Close()
+	return f.Name()
+}
+
 // newWatcher initializes an fsnotify Watcher instance.
 func newWatcher(t *testing.T) *Watcher {
 	watcher, err := NewWatcher()
@@ -1172,6 +1182,50 @@ func TestClose(t *testing.T) {
 	}
 }
 
+// TestRemoveWithClose tests if one can handle Remove events and, at the same
+// time, close Watcher object without any data races.
+func TestRemoveWithClose(t *testing.T) {
+	testDir := tempMkdir(t)
+	defer os.RemoveAll(testDir)
+
+	const fileN = 200
+	tempFiles := make([]string, 0, fileN)
+	for i := 0; i < fileN; i++ {
+		tempFiles = append(tempFiles, tempMkFile(t, testDir))
+	}
+	watcher := newWatcher(t)
+	if err := watcher.Add(testDir); err != nil {
+		t.Fatalf("Expected no error on Add, got %v", err)
+	}
+	startC, stopC := make(chan struct{}), make(chan struct{})
+	errC := make(chan error)
+	go func() {
+		for {
+			select {
+			case <-watcher.Errors:
+			case <-watcher.Events:
+			case <-stopC:
+				return
+			}
+		}
+	}()
+	go func() {
+		<-startC
+		for _, fileName := range tempFiles {
+			os.Remove(fileName)
+		}
+	}()
+	go func() {
+		<-startC
+		errC <- watcher.Close()
+	}()
+	close(startC)
+	defer close(stopC)
+	if err := <-errC; 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 9662a50..7a7700b 100644
--- a/kqueue.go
+++ b/kqueue.go
@@ -72,12 +72,17 @@ func (w *Watcher) Close() error {
 	w.isClosed = true
 	w.mu.Unlock()
 
+	// copy paths to remove while locked
 	w.mu.Lock()
-	ws := w.watches
+	var pathsToRemove = make([]string, 0, len(w.watches))
+	for name := range w.watches {
+		pathsToRemove = append(pathsToRemove, name)
+	}
 	w.mu.Unlock()
+	// unlock before calling Remove, which also locks
 
 	var err error
-	for name := range ws {
+	for _, name := range pathsToRemove {
 		if e := w.Remove(name); e != nil && err == nil {
 			err = e
 		}