diff --git a/CHANGELOG.md b/CHANGELOG.md index 28550f6e..ea60130d 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 0d87631d..721c67e0 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 @@ -552,10 +554,10 @@ func (w *Watcher) associateFile(path string, stat os.FileInfo, follow bool) erro } } // FILE_NOFOLLOW means we watch symlinks themselves rather than their targets - events := unix.FILE_MODIFIED|unix.FILE_ATTRIB|unix.FILE_NOFOLLOW + events := unix.FILE_MODIFIED | unix.FILE_ATTRIB | unix.FILE_NOFOLLOW if follow { // We *DO* follow symlinks for explicitly watched entries - events = unix.FILE_MODIFIED|unix.FILE_ATTRIB + events = unix.FILE_MODIFIED | unix.FILE_ATTRIB } return w.port.AssociatePath(path, stat, events, @@ -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 54c77fbb..75736ce7 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 29087469..edf9aace 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 a9bb1c3c..9e2d082a 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 ae392867..fe09be87 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 30a5bf0f..555a02bb 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 (op Op) String() string { diff --git a/fsnotify_test.go b/fsnotify_test.go index ad322d35..ffec49a8 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) { diff --git a/mkdoc.zsh b/mkdoc.zsh index b09ef768..664ec549 100755 --- a/mkdoc.zsh +++ b/mkdoc.zsh @@ -87,6 +87,8 @@ add=$(<