Skip to content

Commit

Permalink
inotify: fix race in Close() (#465)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
arp242 committed Jul 22, 2022
1 parent 35b6378 commit 7fe2936
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 8 deletions.
3 changes: 3 additions & 0 deletions inotify.go
Expand Up @@ -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()
Expand Down
18 changes: 10 additions & 8 deletions inotify_test.go
Expand Up @@ -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()
Expand Down
15 changes: 15 additions & 0 deletions integration_test.go
Expand Up @@ -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":
Expand Down

0 comments on commit 7fe2936

Please sign in to comment.