From: Chris Howey Date: Sat, 19 Oct 2013 15:38:20 +0000 (-0500) Subject: Add the test from Issue #71 and fix it. X-Git-Tag: v1.7.2~336 X-Git-Url: https://go.fuhry.dev/?a=commitdiff_plain;h=ba28565193315641e3c44fea039f24fb5d4b239e;p=fsnotify.git Add the test from Issue #71 and fix it. The issue was that files watched internally were not being removed when the parent directory's watch was removed. --- diff --git a/fsnotify_bsd.go b/fsnotify_bsd.go index bd4d9d8..5d909e5 100644 --- a/fsnotify_bsd.go +++ b/fsnotify_bsd.go @@ -54,26 +54,28 @@ func (e *FileEvent) IsModify() bool { func (e *FileEvent) IsRename() bool { return (e.mask & sys_NOTE_RENAME) == sys_NOTE_RENAME } type Watcher struct { - mu sync.Mutex // Mutex for the Watcher itself. - kq int // File descriptor (as returned by the kqueue() syscall) - watches map[string]int // Map of watched file diescriptors (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) - finfo map[int]os.FileInfo // Map of file information (isDir, isReg; key: watch descriptor) - pmut sync.Mutex // Protects access to paths and finfo. - fileExists map[string]bool // Keep track of if we know this file exists (to stop duplicate create events) - femut sync.Mutex // Proctects access to fileExists. - 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 - kbuf [1]syscall.Kevent_t // An event buffer for Add/Remove watch - bufmut sync.Mutex // Protects access to kbuf. + mu sync.Mutex // Mutex for the Watcher itself. + kq int // File descriptor (as returned by the kqueue() syscall) + watches map[string]int // Map of watched file diescriptors (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) + finfo map[int]os.FileInfo // Map of file information (isDir, isReg; key: watch descriptor) + pmut sync.Mutex // Protects access to paths and finfo. + fileExists map[string]bool // Keep track of if we know this file exists (to stop duplicate create events) + femut sync.Mutex // Proctects access to fileExists. + externalWatches map[string]bool // Map of watches added by user of the library. + ewmut sync.Mutex // Protects access to internalWatches. + 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 + kbuf [1]syscall.Kevent_t // An event buffer for Add/Remove watch + bufmut sync.Mutex // Protects access to kbuf. } // NewWatcher creates and returns a new kevent instance using kqueue(2) @@ -83,17 +85,18 @@ func NewWatcher() (*Watcher, error) { return nil, os.NewSyscallError("kqueue", errno) } 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), - internalEvent: make(chan *FileEvent), - Event: make(chan *FileEvent), - Error: make(chan error), - done: make(chan bool, 1), + 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() @@ -224,6 +227,9 @@ func (w *Watcher) addWatch(path string, flags uint32) error { // Watch adds path to the watched file set, watching all events. func (w *Watcher) watch(path string) error { + w.ewmut.Lock() + w.externalWatches[path] = true + w.ewmut.Unlock() return w.addWatch(path, sys_NOTE_ALLEVENTS) } @@ -236,7 +242,6 @@ func (w *Watcher) removeWatch(path string) error { return errors.New(fmt.Sprintf("can't remove non-existent kevent watch for: %s", path)) } w.bufmut.Lock() - defer w.bufmut.Unlock() watchEntry := &w.kbuf[0] syscall.SetKevent(watchEntry, watchfd, syscall.EVFILT_VNODE, syscall.EV_DELETE) success, errno := syscall.Kevent(w.kq, w.kbuf[:], nil, nil) @@ -245,6 +250,7 @@ func (w *Watcher) removeWatch(path string) error { } else if (watchEntry.Flags & syscall.EV_ERROR) == syscall.EV_ERROR { return errors.New("kevent rm error") } + w.bufmut.Unlock() syscall.Close(watchfd) w.wmut.Lock() delete(w.watches, path) @@ -254,8 +260,33 @@ func (w *Watcher) removeWatch(path string) error { w.enmut.Unlock() w.pmut.Lock() delete(w.paths, watchfd) + fInfo := w.finfo[watchfd] delete(w.finfo, watchfd) w.pmut.Unlock() + + // Find all watched paths that are in this directory that are not external. + if fInfo.IsDir() { + pathsToRemove := make([]string, 0) + w.pmut.Lock() + for _, wpath := range w.paths { + wdir, _ := filepath.Split(wpath) + if filepath.Clean(wdir) == filepath.Clean(path) { + w.ewmut.Lock() + if _, extern := w.externalWatches[wpath]; !extern { + pathsToRemove = append(pathsToRemove, wpath) + } + w.ewmut.Unlock() + } + } + w.pmut.Unlock() + for idx := 0; idx < len(pathsToRemove); idx++ { + // Since these are internal, not much sense in propogating error + // to the user, as that will just confuse them with an error about + // a path they did not explicitly watch themselves. + w.removeWatch(pathsToRemove[idx]) + } + } + return nil } diff --git a/fsnotify_test.go b/fsnotify_test.go index e5346d1..9e39f01 100644 --- a/fsnotify_test.go +++ b/fsnotify_test.go @@ -896,6 +896,64 @@ func TestFsnotifyRenameToOverwrite(t *testing.T) { os.Remove(testFileRenamed) } +func TestRemovalOfWatch(t *testing.T) { + var testDir string = testTempDir() + + // Create directory to watch + if err := os.Mkdir(testDir, 0777); err != nil { + t.Fatalf("failed to create test directory: %s", err) + } + defer os.RemoveAll(testDir) + + // Create a file before watching directory + var testFileAlreadyExists string = filepath.Join(testDir, "TestFsnotifyEventsExisting.testfile") + { + var f *os.File + f, err := os.OpenFile(testFileAlreadyExists, os.O_WRONLY|os.O_CREATE, 0666) + if err != nil { + t.Fatalf("creating test file failed: %s", err) + } + f.Sync() + f.Close() + } + + watcher, err := NewWatcher() + if err != nil { + t.Fatalf("NewWatcher() failed: %s", err) + } + defer watcher.Close() + + watcher.Watch(testDir) + err = watcher.RemoveWatch(testDir) + if err != nil { + t.Fatalf("Could not remove the watch: %v\n", err) + } + + go func() { + select { + case ev := <-watcher.Event: + t.Fatalf("We received event: %v\n", ev) + case <-time.After(500 * time.Millisecond): + t.Log("No event received, as expected.") + } + }() + + time.Sleep(200 * time.Millisecond) + // Modify the file outside of the watched dir + f, err := os.Open(testFileAlreadyExists) + if err != nil { + t.Fatalf("Open test file failed: %s", err) + } + f.WriteString("data") + f.Sync() + f.Close() + err = os.Chmod(testFileAlreadyExists, 0700) + if err != nil { + t.Fatalf("chmod failed: %s", err) + } + time.Sleep(400 * time.Millisecond) +} + func TestFsnotifyAttrib(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("attributes don't work on Windows.")