From d9e1c40664a23dd5cf6101f6dbdba1bcd2d8839c Mon Sep 17 00:00:00 2001 From: Nathan John Youngman Date: Fri, 20 Jun 2014 14:45:55 +0900 Subject: [PATCH] go.exp/fsnotify: remove current implementation of WatchFlags removes fsnFlags, purgeEvents and internalEvent channel The current implementation: * doesn't take advantage of OS for efficiency * no benefit over filtering events as received * extra bookkeeping and mutexes * no tests * not correctly implemented on Windows https://github.com/howeyc/fsnotify/issues/93#issuecomment-39285195 LGTM=mikioh.mikioh, rsc R=golang-codereviews, mikioh.mikioh, rsc CC=golang-codereviews https://golang.org/cl/100860043 --- fsnotify/fsnotify.go | 64 ------------------------------------ fsnotify/fsnotify_bsd.go | 30 ++--------------- fsnotify/fsnotify_linux.go | 54 ++++++++++-------------------- fsnotify/fsnotify_windows.go | 42 ++++++++++------------- 4 files changed, 37 insertions(+), 153 deletions(-) diff --git a/fsnotify/fsnotify.go b/fsnotify/fsnotify.go index 6d1ccbeb3..2622de407 100644 --- a/fsnotify/fsnotify.go +++ b/fsnotify/fsnotify.go @@ -7,77 +7,13 @@ package fsnotify import "fmt" -const ( - FSN_CREATE = 1 - FSN_MODIFY = 2 - FSN_DELETE = 4 - FSN_RENAME = 8 - - FSN_ALL = FSN_MODIFY | FSN_DELETE | FSN_RENAME | FSN_CREATE -) - -// Purge events from interal chan to external chan if passes filter -func (w *Watcher) purgeEvents() { - for ev := range w.internalEvent { - sendEvent := false - w.fsnmut.Lock() - fsnFlags := w.fsnFlags[ev.Name] - w.fsnmut.Unlock() - - if (fsnFlags&FSN_CREATE == FSN_CREATE) && ev.IsCreate() { - sendEvent = true - } - - if (fsnFlags&FSN_MODIFY == FSN_MODIFY) && ev.IsModify() { - sendEvent = true - } - - if (fsnFlags&FSN_DELETE == FSN_DELETE) && ev.IsDelete() { - sendEvent = true - } - - if (fsnFlags&FSN_RENAME == FSN_RENAME) && ev.IsRename() { - sendEvent = true - } - - if sendEvent { - w.Event <- ev - } - - // If there's no file, then no more events for user - // BSD must keep watch for internal use (watches DELETEs to keep track - // what files exist for create events) - if ev.IsDelete() { - w.fsnmut.Lock() - delete(w.fsnFlags, ev.Name) - w.fsnmut.Unlock() - } - } - - close(w.Event) -} - // Watch a given file path func (w *Watcher) Watch(path string) error { - w.fsnmut.Lock() - w.fsnFlags[path] = FSN_ALL - w.fsnmut.Unlock() - return w.watch(path) -} - -// Watch a given file path for a particular set of notifications (FSN_MODIFY etc.) -func (w *Watcher) WatchFlags(path string, flags uint32) error { - w.fsnmut.Lock() - w.fsnFlags[path] = flags - w.fsnmut.Unlock() return w.watch(path) } // Remove a watch on a file func (w *Watcher) RemoveWatch(path string) error { - w.fsnmut.Lock() - delete(w.fsnFlags, path) - w.fsnmut.Unlock() return w.removeWatch(path) } diff --git a/fsnotify/fsnotify_bsd.go b/fsnotify/fsnotify_bsd.go index 7fb24af80..1941f9b66 100644 --- a/fsnotify/fsnotify_bsd.go +++ b/fsnotify/fsnotify_bsd.go @@ -63,8 +63,6 @@ type Watcher struct { kq int // File descriptor (as returned by the kqueue() syscall) watches map[string]int // Map of watched file descriptors (key: path) wmut sync.Mutex // Protects access to watches. - fsnFlags map[string]uint32 // Map of watched files to flags used for filter - fsnmut sync.Mutex // Protects access to fsnFlags. enFlags map[string]uint32 // Map of watched files to evfilt note flags used in kqueue enmut sync.Mutex // Protects access to enFlags. paths map[int]string // Map of watched paths (key: watch descriptor) @@ -75,7 +73,6 @@ type Watcher struct { externalWatches map[string]bool // Map of watches added by user of the library. ewmut sync.Mutex // Protects access to externalWatches. Error chan error // Errors are sent on this channel - internalEvent chan *FileEvent // Events are queued on this channel Event chan *FileEvent // Events are returned on this channel done chan bool // Channel for sending a "quit message" to the reader goroutine isClosed bool // Set to true when Close() is first called @@ -92,20 +89,17 @@ func NewWatcher() (*Watcher, error) { w := &Watcher{ kq: fd, watches: make(map[string]int), - fsnFlags: make(map[string]uint32), enFlags: make(map[string]uint32), paths: make(map[int]string), finfo: make(map[int]os.FileInfo), fileExists: make(map[string]bool), externalWatches: make(map[string]bool), - internalEvent: make(chan *FileEvent), Event: make(chan *FileEvent), Error: make(chan error), done: make(chan bool, 1), } go w.readEvents() - go w.purgeEvents() return w, nil } @@ -323,7 +317,7 @@ func (w *Watcher) readEvents() { if errno != nil { w.Error <- os.NewSyscallError("close", errno) } - close(w.internalEvent) + close(w.Event) close(w.Error) return } @@ -369,7 +363,7 @@ func (w *Watcher) readEvents() { w.sendDirectoryChangeEvents(fileEvent.Name) } else { // Send the event on the events channel - w.internalEvent <- fileEvent + w.Event <- fileEvent } // Move to next event @@ -419,15 +413,6 @@ func (w *Watcher) watchDirectoryFiles(dirPath string) error { for _, fileInfo := range files { filePath := filepath.Join(dirPath, fileInfo.Name()) - // Inherit fsnFlags from parent directory - w.fsnmut.Lock() - if flags, found := w.fsnFlags[dirPath]; found { - w.fsnFlags[filePath] = flags - } else { - w.fsnFlags[filePath] = FSN_ALL - } - w.fsnmut.Unlock() - if fileInfo.IsDir() == false { // Watch file to mimic linux fsnotify e := w.addWatch(filePath, sys_NOTE_ALLEVENTS) @@ -477,20 +462,11 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) { _, doesExist := w.fileExists[filePath] w.femut.Unlock() if !doesExist { - // Inherit fsnFlags from parent directory - w.fsnmut.Lock() - if flags, found := w.fsnFlags[dirPath]; found { - w.fsnFlags[filePath] = flags - } else { - w.fsnFlags[filePath] = FSN_ALL - } - w.fsnmut.Unlock() - // Send create event fileEvent := new(FileEvent) fileEvent.Name = filePath fileEvent.create = true - w.internalEvent <- fileEvent + w.Event <- fileEvent } w.femut.Lock() w.fileExists[filePath] = true diff --git a/fsnotify/fsnotify_linux.go b/fsnotify/fsnotify_linux.go index 80ade879f..8dead7307 100644 --- a/fsnotify/fsnotify_linux.go +++ b/fsnotify/fsnotify_linux.go @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build linux - package fsnotify import ( @@ -93,17 +91,14 @@ type watch struct { } type Watcher struct { - mu sync.Mutex // Map access - fd int // File descriptor (as returned by the inotify_init() syscall) - watches map[string]*watch // Map of inotify watches (key: path) - fsnFlags map[string]uint32 // Map of watched files to flags used for filter - fsnmut sync.Mutex // Protects access to fsnFlags. - paths map[int]string // Map of watched paths (key: watch descriptor) - Error chan error // Errors are sent on this channel - internalEvent chan *FileEvent // Events are queued on this channel - Event chan *FileEvent // Events are returned on this channel - done chan bool // Channel for sending a "quit message" to the reader goroutine - isClosed bool // Set to true when Close() is first called + mu sync.Mutex // Map access + fd int // File descriptor (as returned by the inotify_init() syscall) + watches map[string]*watch // Map of inotify watches (key: path) + paths map[int]string // Map of watched paths (key: watch descriptor) + Error chan error // Errors are sent on this channel + Event chan *FileEvent // Events are returned on this channel + done chan bool // Channel for sending a "quit message" to the reader goroutine + isClosed bool // Set to true when Close() is first called } // NewWatcher creates and returns a new inotify instance using inotify_init(2) @@ -113,18 +108,15 @@ func NewWatcher() (*Watcher, error) { return nil, os.NewSyscallError("inotify_init", errno) } w := &Watcher{ - fd: fd, - watches: make(map[string]*watch), - fsnFlags: make(map[string]uint32), - paths: make(map[int]string), - internalEvent: make(chan *FileEvent), - Event: make(chan *FileEvent), - Error: make(chan error), - done: make(chan bool, 1), + fd: fd, + watches: make(map[string]*watch), + paths: make(map[int]string), + Event: make(chan *FileEvent), + Error: make(chan error), + done: make(chan bool, 1), } go w.readEvents() - go w.purgeEvents() return w, nil } @@ -210,7 +202,7 @@ func (w *Watcher) readEvents() { select { case <-w.done: syscall.Close(w.fd) - close(w.internalEvent) + close(w.Event) close(w.Error) return default: @@ -221,7 +213,7 @@ func (w *Watcher) readEvents() { // If EOF is received if n == 0 { syscall.Close(w.fd) - close(w.internalEvent) + close(w.Event) close(w.Error) return } @@ -252,7 +244,6 @@ func (w *Watcher) readEvents() { w.mu.Lock() event.Name = w.paths[int(raw.Wd)] w.mu.Unlock() - watchedName := event.Name if nameLen > 0 { // Point "bytes" at the first byte of the filename bytes := (*[syscall.PathMax]byte)(unsafe.Pointer(&buf[offset+syscall.SizeofInotifyEvent])) @@ -262,18 +253,7 @@ func (w *Watcher) readEvents() { // Send the events that are not ignored on the events channel if !event.ignoreLinux() { - // Setup FSNotify flags (inherit from directory watch) - w.fsnmut.Lock() - if _, fsnFound := w.fsnFlags[event.Name]; !fsnFound { - if fsnFlags, watchFound := w.fsnFlags[watchedName]; watchFound { - w.fsnFlags[event.Name] = fsnFlags - } else { - w.fsnFlags[event.Name] = FSN_ALL - } - } - w.fsnmut.Unlock() - - w.internalEvent <- event + w.Event <- event } // Move to the next event in the buffer diff --git a/fsnotify/fsnotify_windows.go b/fsnotify/fsnotify_windows.go index d88ae6340..78125eff8 100644 --- a/fsnotify/fsnotify_windows.go +++ b/fsnotify/fsnotify_windows.go @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build windows - package fsnotify import ( @@ -115,18 +113,15 @@ type watchMap map[uint32]indexMap // A Watcher waits for and receives event notifications // for a specific set of files and directories. type Watcher struct { - mu sync.Mutex // Map access - port syscall.Handle // Handle to completion port - watches watchMap // Map of watches (key: i-number) - fsnFlags map[string]uint32 // Map of watched files to flags used for filter - fsnmut sync.Mutex // Protects access to fsnFlags. - input chan *input // Inputs to the reader are sent on this channel - internalEvent chan *FileEvent // Events are queued on this channel - Event chan *FileEvent // Events are returned on this channel - Error chan error // Errors are sent on this channel - isClosed bool // Set to true when Close() is first called - quit chan chan<- error - cookie uint32 + mu sync.Mutex // Map access + port syscall.Handle // Handle to completion port + watches watchMap // Map of watches (key: i-number) + input chan *input // Inputs to the reader are sent on this channel + Event chan *FileEvent // Events are returned on this channel + Error chan error // Errors are sent on this channel + isClosed bool // Set to true when Close() is first called + quit chan chan<- error + cookie uint32 } // NewWatcher creates and returns a Watcher. @@ -136,17 +131,14 @@ func NewWatcher() (*Watcher, error) { return nil, os.NewSyscallError("CreateIoCompletionPort", e) } w := &Watcher{ - port: port, - watches: make(watchMap), - fsnFlags: make(map[string]uint32), - input: make(chan *input, 1), - Event: make(chan *FileEvent, 50), - internalEvent: make(chan *FileEvent), - Error: make(chan error), - quit: make(chan chan<- error, 1), + port: port, + watches: make(watchMap), + input: make(chan *input, 1), + Event: make(chan *FileEvent, 50), + Error: make(chan error), + quit: make(chan chan<- error, 1), } go w.readEvents() - go w.purgeEvents() return w, nil } @@ -431,7 +423,7 @@ func (w *Watcher) readEvents() { if e := syscall.CloseHandle(w.port); e != nil { err = os.NewSyscallError("CloseHandle", e) } - close(w.internalEvent) + close(w.Event) close(w.Error) ch <- err return @@ -475,7 +467,7 @@ func (w *Watcher) readEvents() { var offset uint32 for { if n == 0 { - w.internalEvent <- &FileEvent{mask: sys_FS_Q_OVERFLOW} + w.Event <- &FileEvent{mask: sys_FS_Q_OVERFLOW} w.Error <- errors.New("short read in readEvents()") break }