From 7fe2936a4fe9f4eda1299c36dda9e12d4bcab6c7 Mon Sep 17 00:00:00 2001
From: Martin Tournoij <martin@arp242.net>
Date: Fri, 22 Jul 2022 07:22:49 +0200
Subject: [PATCH] inotify: fix race in Close() (#465)

Would sometimes fail with:

	panic: close of closed channel

	goroutine 204 [running]:
	github.com/fsnotify/fsnotify.(*Watcher).Close(0xc0003e6410)
        	/home/martin/code/Golib/fsnotify/inotify.go:82 +0x66
	created by github.com/fsnotify/fsnotify.TestCloseRace
        	/home/martin/code/Golib/fsnotify/integration_test.go:1256 +0x1a5
	exit status 2

Because isClosed() might return "false" for two goroutines, and then
they will both try to close it, resulting in the panic.

Fixes #367
---
 inotify.go          |  3 +++
 inotify_test.go     | 18 ++++++++++--------
 integration_test.go | 15 +++++++++++++++
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/inotify.go b/inotify.go
index 9e020ed4..b01124a6 100644
--- a/inotify.go
+++ b/inotify.go
@@ -72,12 +72,15 @@ func (w *Watcher) isClosed() bool {
 
 // Close removes all watches and closes the events channel.
 func (w *Watcher) Close() error {
+	w.mu.Lock()
 	if w.isClosed() {
+		w.mu.Unlock()
 		return nil
 	}
 
 	// Send 'close' signal to goroutine, and set the Watcher to closed.
 	close(w.done)
+	w.mu.Unlock()
 
 	// Wake up goroutine
 	w.poller.wake()
diff --git a/inotify_test.go b/inotify_test.go
index 361f1982..269c3ff8 100644
--- a/inotify_test.go
+++ b/inotify_test.go
@@ -354,14 +354,16 @@ func TestInotifyInnerMapLength(t *testing.T) {
 	_ = <-w.Events                      // consume Remove event
 	<-time.After(50 * time.Millisecond) // wait IN_IGNORE propagated
 
-	w.mu.Lock()
-	defer w.mu.Unlock()
-	if len(w.watches) != 0 {
-		t.Fatalf("Expected watches len is 0, but got: %d, %v", len(w.watches), w.watches)
-	}
-	if len(w.paths) != 0 {
-		t.Fatalf("Expected paths len is 0, but got: %d, %v", len(w.paths), w.paths)
-	}
+	func() {
+		w.mu.Lock()
+		defer w.mu.Unlock()
+		if len(w.watches) != 0 {
+			t.Fatalf("Expected watches len is 0, but got: %d, %v", len(w.watches), w.watches)
+		}
+		if len(w.paths) != 0 {
+			t.Fatalf("Expected paths len is 0, but got: %d, %v", len(w.paths), w.paths)
+		}
+	}()
 
 	w.Close()
 	wg.Wait()
diff --git a/integration_test.go b/integration_test.go
index 9dace27b..fefae5ea 100644
--- a/integration_test.go
+++ b/integration_test.go
@@ -1244,6 +1244,21 @@ func TestRemoveWithClose(t *testing.T) {
 	}
 }
 
+// Make sure Close() doesn't race; hard to write a good reproducible test for
+// this, but running it 150 times seems to reproduce it in ~75% of cases and
+// isn't too slow (~0.06s on my system).
+func TestCloseRace(t *testing.T) {
+	for i := 0; i < 150; i++ {
+		w, err := NewWatcher()
+		if err != nil {
+			t.Fatal(err)
+		}
+		go w.Close()
+		go w.Close()
+		go w.Close()
+	}
+}
+
 func testRename(file1, file2 string) error {
 	switch runtime.GOOS {
 	case "windows", "plan9":