Skip to content

Commit

Permalink
Return ErrClosed on Add() when the watcher is closed
Browse files Browse the repository at this point in the history
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
  • Loading branch information
arp242 committed Oct 13, 2022
1 parent acadbbe commit 42e2369
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 10 deletions.
18 changes: 13 additions & 5 deletions backend_fen.go
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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()

Expand Down
14 changes: 13 additions & 1 deletion backend_inotify.go
Expand Up @@ -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
Expand All @@ -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 |
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()

Expand Down
13 changes: 12 additions & 1 deletion backend_kqueue.go
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions backend_other.go
Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion backend_windows.go
Expand Up @@ -199,6 +199,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
Expand All @@ -220,7 +222,7 @@ func (w *Watcher) Add(name string) error {
w.mu.Lock()
if w.isClosed {
w.mu.Unlock()
return errors.New("watcher already closed")
return ErrClosed
}
w.mu.Unlock()

Expand All @@ -244,6 +246,13 @@ 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 {
w.mu.Lock()
if w.isClosed {
w.mu.Unlock()
return nil
}
w.mu.Unlock()

in := &input{
op: opRemoveWatch,
path: filepath.Clean(name),
Expand All @@ -257,9 +266,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.watches))
for _, entry := range w.watches {
Expand Down
5 changes: 3 additions & 2 deletions fsnotify.go
Expand Up @@ -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 {
Expand Down
22 changes: 22 additions & 0 deletions fsnotify_test.go
Expand Up @@ -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 ugh := w.WatchList(); ugh != nil { // This function is crap.
t.Fatalf("WatchList not nil: %#v", ugh)
}
})
}

func TestAdd(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions mkdoc.zsh
Expand Up @@ -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
Expand Down Expand Up @@ -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
)

Expand Down

0 comments on commit 42e2369

Please sign in to comment.