Repository: fsnotify/fsnotify (go)

Issue:
Make watcher `isClosed` public
**Is your feature request related to a problem? Please describe.**
In some cases I am trying to add files to the watcher while the watcher has already been closed. Instead of writing logic on the (OS dependent) error message I receive, I want to check whether the watcher has been closed myself before adding files.

**Describe the solution you'd like**
Make `Watcher.isClosed` public.

**Describe alternatives you've considered**
```
...
err := watcher.Add(path)
if err.Error() == "inotify instance already closed" || err.Error() == "watcher already closed" { // OS dependent
    ...
}
```

**Additional context**
Add any other context or screenshots about the feature request here.


Pull request:
Return ErrClosed on Add() when the watcher is closed
Also consistently return nil for Remove() and WatchList(). For Remove() we could return an error, but I don't really see the point: if the watcher is closed it's basically a no-op so returning nil is fine.

For WatchList() I'd like to return an error, but there is no error return, so explicitly returning nil is the best we can do.

Fixes #511

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 28550f6..ea60130 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,9 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Additions
 
-- Add FEN backend to support illumos and Solaris. ([#371])
+- illumos: add FEN backend to support illumos and Solaris. ([#371])
+
+### Changes and fixes
+
+- all: return ErrClosed on Add() when the watcher is closed ([#516])
 
 [#371]: https://github.com/fsnotify/fsnotify/pull/371
+[#516]: https://github.com/fsnotify/fsnotify/pull/516
 
 ## [1.6.0] - 2022-10-13
 
diff --git a/backend_fen.go b/backend_fen.go
index 4d12812..47372e5 100644
--- a/backend_fen.go
+++ b/backend_fen.go
@@ -196,6 +196,8 @@ func (w *Watcher) Close() error {
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
 //
+// Returns [ErrClosed] if [Watcher.Close] was called.
+//
 // # Watching directories
 //
 // All files in a directory are monitored, including new files that are created
@@ -215,7 +217,7 @@ func (w *Watcher) Close() error {
 // you're not interested in. There is an example of this in [cmd/fsnotify/file.go].
 func (w *Watcher) Add(name string) error {
 	if w.isClosed() {
-		return errors.New("FEN watcher already closed")
+		return ErrClosed
 	}
 	if w.port.PathIsWatched(name) {
 		return nil
@@ -260,7 +262,7 @@ func (w *Watcher) Add(name string) error {
 // Removing a path that has not yet been added returns [ErrNonExistentWatch].
 func (w *Watcher) Remove(name string) error {
 	if w.isClosed() {
-		return errors.New("FEN watcher already closed")
+		return nil
 	}
 	if !w.port.PathIsWatched(name) {
 		return fmt.Errorf("%w: %s", ErrNonExistentWatch, name)
@@ -528,7 +530,7 @@ func (w *Watcher) updateDirectory(path string) error {
 
 func (w *Watcher) associateFile(path string, stat os.FileInfo, follow bool) error {
 	if w.isClosed() {
-		return errors.New("FEN watcher already closed")
+		return ErrClosed
 	}
 	// This is primarily protecting the call to AssociatePath
 	// but it is important and intentional that the call to
@@ -570,7 +572,13 @@ func (w *Watcher) dissociateFile(path string, stat os.FileInfo, unused bool) err
 }
 
 // WatchList returns all paths added with [Add] (and are not yet removed).
+//
+// Returns nil if [Watcher.Close] was called.
 func (w *Watcher) WatchList() []string {
+	if w.isClosed() {
+		return nil
+	}
+
 	w.mu.Lock()
 	defer w.mu.Unlock()
 
diff --git a/backend_inotify.go b/backend_inotify.go
index 54c77fb..75736ce 100644
--- a/backend_inotify.go
+++ b/backend_inotify.go
@@ -218,6 +218,8 @@ func (w *Watcher) Close() error {
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
 //
+// Returns [ErrClosed] if [Watcher.Close] was called.
+//
 // # Watching directories
 //
 // All files in a directory are monitored, including new files that are created
@@ -238,7 +240,7 @@ func (w *Watcher) Close() error {
 func (w *Watcher) Add(name string) error {
 	name = filepath.Clean(name)
 	if w.isClosed() {
-		return errors.New("inotify instance already closed")
+		return ErrClosed
 	}
 
 	var flags uint32 = unix.IN_MOVED_TO | unix.IN_MOVED_FROM |
@@ -274,6 +276,10 @@ func (w *Watcher) Add(name string) error {
 //
 // Removing a path that has not yet been added returns [ErrNonExistentWatch].
 func (w *Watcher) Remove(name string) error {
+	if w.isClosed() {
+		return nil
+	}
+
 	name = filepath.Clean(name)
 
 	// Fetch the watch.
@@ -317,7 +323,13 @@ func (w *Watcher) Remove(name string) error {
 }
 
 // WatchList returns all paths added with [Add] (and are not yet removed).
+//
+// Returns nil if [Watcher.Close] was called.
 func (w *Watcher) WatchList() []string {
+	if w.isClosed() {
+		return nil
+	}
+
 	w.mu.Lock()
 	defer w.mu.Unlock()
 
diff --git a/backend_kqueue.go b/backend_kqueue.go
index 2908746..edf9aac 100644
--- a/backend_kqueue.go
+++ b/backend_kqueue.go
@@ -250,6 +250,8 @@ func (w *Watcher) Close() error {
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
 //
+// Returns [ErrClosed] if [Watcher.Close] was called.
+//
 // # Watching directories
 //
 // All files in a directory are monitored, including new files that are created
@@ -284,6 +286,10 @@ func (w *Watcher) Add(name string) error {
 func (w *Watcher) Remove(name string) error {
 	name = filepath.Clean(name)
 	w.mu.Lock()
+	if w.isClosed {
+		w.mu.Unlock()
+		return nil
+	}
 	watchfd, ok := w.watches[name]
 	w.mu.Unlock()
 	if !ok {
@@ -337,9 +343,14 @@ func (w *Watcher) Remove(name string) error {
 }
 
 // WatchList returns all paths added with [Add] (and are not yet removed).
+//
+// Returns nil if [Watcher.Close] was called.
 func (w *Watcher) WatchList() []string {
 	w.mu.Lock()
 	defer w.mu.Unlock()
+	if w.isClosed {
+		return nil
+	}
 
 	entries := make([]string, 0, len(w.userWatches))
 	for pathname := range w.userWatches {
@@ -363,7 +374,7 @@ func (w *Watcher) addWatch(name string, flags uint32) (string, error) {
 	w.mu.Lock()
 	if w.isClosed {
 		w.mu.Unlock()
-		return "", errors.New("kevent instance already closed")
+		return "", ErrClosed
 	}
 	watchfd, alreadyWatching := w.watches[name]
 	// We already have a watch, but we can still override flags.
diff --git a/backend_other.go b/backend_other.go
index a9bb1c3..9e2d082 100644
--- a/backend_other.go
+++ b/backend_other.go
@@ -34,6 +34,8 @@ func (w *Watcher) Close() error {
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
 //
+// Returns [ErrClosed] if [Watcher.Close] was called.
+//
 // # Watching directories
 //
 // All files in a directory are monitored, including new files that are created
diff --git a/backend_windows.go b/backend_windows.go
index ae39286..fe09be8 100644
--- a/backend_windows.go
+++ b/backend_windows.go
@@ -120,9 +120,9 @@ type Watcher struct {
 	input chan *input    // Inputs to the reader are sent on this channel
 	quit  chan chan<- error
 
-	mu       sync.Mutex // Protects access to watches, isClosed
-	watches  watchMap   // Map of watches (key: i-number)
-	isClosed bool       // Set to true when Close() is first called
+	mu      sync.Mutex // Protects access to watches, closed
+	watches watchMap   // Map of watches (key: i-number)
+	closed  bool       // Set to true when Close() is first called
 }
 
 // NewWatcher creates a new Watcher.
@@ -143,6 +143,12 @@ func NewWatcher() (*Watcher, error) {
 	return w, nil
 }
 
+func (w *Watcher) isClosed() bool {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+	return w.closed
+}
+
 func (w *Watcher) sendEvent(name string, mask uint64) bool {
 	if mask == 0 {
 		return false
@@ -169,12 +175,12 @@ func (w *Watcher) sendError(err error) bool {
 
 // Close removes all watches and closes the events channel.
 func (w *Watcher) Close() error {
-	w.mu.Lock()
-	if w.isClosed {
-		w.mu.Unlock()
+	if w.isClosed() {
 		return nil
 	}
-	w.isClosed = true
+
+	w.mu.Lock()
+	w.closed = true
 	w.mu.Unlock()
 
 	// Send "quit" message to the reader goroutine
@@ -199,6 +205,8 @@ func (w *Watcher) Close() error {
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
 //
+// Returns [ErrClosed] if [Watcher.Close] was called.
+//
 // # Watching directories
 //
 // All files in a directory are monitored, including new files that are created
@@ -217,12 +225,9 @@ func (w *Watcher) Close() error {
 // Instead, watch the parent directory and use Event.Name to filter out files
 // you're not interested in. There is an example of this in [cmd/fsnotify/file.go].
 func (w *Watcher) Add(name string) error {
-	w.mu.Lock()
-	if w.isClosed {
-		w.mu.Unlock()
-		return errors.New("watcher already closed")
+	if w.isClosed() {
+		return ErrClosed
 	}
-	w.mu.Unlock()
 
 	in := &input{
 		op:    opAddWatch,
@@ -244,6 +249,10 @@ func (w *Watcher) Add(name string) error {
 //
 // Removing a path that has not yet been added returns [ErrNonExistentWatch].
 func (w *Watcher) Remove(name string) error {
+	if w.isClosed() {
+		return nil
+	}
+
 	in := &input{
 		op:    opRemoveWatch,
 		path:  filepath.Clean(name),
@@ -257,7 +266,13 @@ func (w *Watcher) Remove(name string) error {
 }
 
 // WatchList returns all paths added with [Add] (and are not yet removed).
+//
+// Returns nil if [Watcher.Close] was called.
 func (w *Watcher) WatchList() []string {
+	if w.isClosed() {
+		return nil
+	}
+
 	w.mu.Lock()
 	defer w.mu.Unlock()
 
diff --git a/fsnotify.go b/fsnotify.go
index 7bcf765..c93b5b8 100644
--- a/fsnotify.go
+++ b/fsnotify.go
@@ -42,8 +42,9 @@ const (
 
 // Common errors that can be reported by a watcher
 var (
-	ErrNonExistentWatch = errors.New("can't remove non-existent watcher")
-	ErrEventOverflow    = errors.New("fsnotify queue overflow")
+	ErrNonExistentWatch = errors.New("fsnotify: can't remove non-existent watcher")
+	ErrEventOverflow    = errors.New("fsnotify: queue overflow")
+	ErrClosed           = errors.New("fsnotify: watcher already closed")
 )
 
 func (o Op) String() string {
diff --git a/mkdoc.zsh b/mkdoc.zsh
index b09ef76..664ec54 100755
--- a/mkdoc.zsh
+++ b/mkdoc.zsh
@@ -87,6 +87,8 @@ add=$(<<EOF
 // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special
 // filesystems (/proc, /sys, etc.) generally don't work.
 //
+// Returns [ErrClosed] if [Watcher.Close] was called.
+//
 // # Watching directories
 //
 // All files in a directory are monitored, including new files that are created
@@ -124,6 +126,8 @@ EOF
 
 watchlist=$(<<EOF
 // WatchList returns all paths added with [Add] (and are not yet removed).
+//
+// Returns nil if [Watcher.Close] was called.
 EOF
 )
 


Test patch:
diff --git a/fsnotify_test.go b/fsnotify_test.go
index ad322d3..ffec49a 100644
--- a/fsnotify_test.go
+++ b/fsnotify_test.go
@@ -892,6 +892,28 @@ func TestClose(t *testing.T) {
 
 		chanClosed(t, w.w)
 	})
+
+	t.Run("error after closed", func(t *testing.T) {
+		t.Parallel()
+
+		tmp := t.TempDir()
+		w := newWatcher(t, tmp)
+		if err := w.Close(); err != nil {
+			t.Fatal(err)
+		}
+
+		file := filepath.Join(tmp, "file")
+		touch(t, file)
+		if err := w.Add(file); !errors.Is(err, ErrClosed) {
+			t.Fatalf("wrong error for Add: %#v", err)
+		}
+		if err := w.Remove(file); err != nil {
+			t.Fatalf("wrong error for Remove: %#v", err)
+		}
+		if l := w.WatchList(); l != nil { // Should return an error, but meh :-/
+			t.Fatalf("WatchList not nil: %#v", l)
+		}
+	})
 }
 
 func TestAdd(t *testing.T) {