Browse code

Update fsnotify to fix deadlock in removing watch

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2017/11/09 06:51:06
Showing 4 changed files
... ...
@@ -85,7 +85,7 @@ github.com/philhofer/fwd 98c11a7a6ec829d672b03833c3d69a7fae1ca972
85 85
 github.com/tinylib/msgp 75ee40d2601edf122ef667e2a07d600d4c44490c
86 86
 
87 87
 # fsnotify
88
-github.com/fsnotify/fsnotify v1.4.2
88
+github.com/fsnotify/fsnotify 4da3e2cfbabc9f751898f250b49f2439785783a1
89 89
 
90 90
 # awslogs deps
91 91
 github.com/aws/aws-sdk-go v1.4.22
... ...
@@ -8,14 +8,14 @@ fsnotify utilizes [golang.org/x/sys](https://godoc.org/golang.org/x/sys) rather
8 8
 go get -u golang.org/x/sys/...
9 9
 ```
10 10
 
11
-Cross platform: Windows, Linux, BSD and OS X.
11
+Cross platform: Windows, Linux, BSD and macOS.
12 12
 
13 13
 |Adapter   |OS        |Status    |
14 14
 |----------|----------|----------|
15 15
 |inotify   |Linux 2.6.27 or later, Android\*|Supported [![Build Status](https://travis-ci.org/fsnotify/fsnotify.svg?branch=master)](https://travis-ci.org/fsnotify/fsnotify)|
16
-|kqueue    |BSD, OS X, iOS\*|Supported [![Build Status](https://travis-ci.org/fsnotify/fsnotify.svg?branch=master)](https://travis-ci.org/fsnotify/fsnotify)|
16
+|kqueue    |BSD, macOS, iOS\*|Supported [![Build Status](https://travis-ci.org/fsnotify/fsnotify.svg?branch=master)](https://travis-ci.org/fsnotify/fsnotify)|
17 17
 |ReadDirectoryChangesW|Windows|Supported [![Build status](https://ci.appveyor.com/api/projects/status/ivwjubaih4r0udeh/branch/master?svg=true)](https://ci.appveyor.com/project/NathanYoungman/fsnotify/branch/master)|
18
-|FSEvents  |OS X          |[Planned](https://github.com/fsnotify/fsnotify/issues/11)|
18
+|FSEvents  |macOS         |[Planned](https://github.com/fsnotify/fsnotify/issues/11)|
19 19
 |FEN       |Solaris 11    |[In Progress](https://github.com/fsnotify/fsnotify/issues/12)|
20 20
 |fanotify  |Linux 2.6.37+ | |
21 21
 |USN Journals |Windows    |[Maybe](https://github.com/fsnotify/fsnotify/issues/53)|
... ...
@@ -23,7 +23,7 @@ Cross platform: Windows, Linux, BSD and OS X.
23 23
 
24 24
 \* Android and iOS are untested.
25 25
 
26
-Please see [the documentation](https://godoc.org/github.com/fsnotify/fsnotify) for usage. Consult the [Wiki](https://github.com/fsnotify/fsnotify/wiki) for the FAQ and further information.
26
+Please see [the documentation](https://godoc.org/github.com/fsnotify/fsnotify) and consult the [FAQ](#faq) for usage information.
27 27
 
28 28
 ## API stability
29 29
 
... ...
@@ -41,6 +41,35 @@ Please refer to [CONTRIBUTING][] before opening an issue or pull request.
41 41
 
42 42
 See [example_test.go](https://github.com/fsnotify/fsnotify/blob/master/example_test.go).
43 43
 
44
+## FAQ
45
+
46
+**When a file is moved to another directory is it still being watched?**
47
+
48
+No (it shouldn't be, unless you are watching where it was moved to).
49
+
50
+**When I watch a directory, are all subdirectories watched as well?**
51
+
52
+No, you must add watches for any directory you want to watch (a recursive watcher is on the roadmap [#18][]).
53
+
54
+**Do I have to watch the Error and Event channels in a separate goroutine?**
55
+
56
+As of now, yes. Looking into making this single-thread friendly (see [howeyc #7][#7])
57
+
58
+**Why am I receiving multiple events for the same file on OS X?**
59
+
60
+Spotlight indexing on OS X can result in multiple events (see [howeyc #62][#62]). A temporary workaround is to add your folder(s) to the *Spotlight Privacy settings* until we have a native FSEvents implementation (see [#11][]).
61
+
62
+**How many files can be watched at once?**
63
+
64
+There are OS-specific limits as to how many watches can be created:
65
+* Linux: /proc/sys/fs/inotify/max_user_watches contains the limit, reaching this limit results in a "no space left on device" error.
66
+* BSD / OSX: sysctl variables "kern.maxfiles" and "kern.maxfilesperproc", reaching these limits results in a "too many open files" error.
67
+
68
+[#62]: https://github.com/howeyc/fsnotify/issues/62
69
+[#18]: https://github.com/fsnotify/fsnotify/issues/18
70
+[#11]: https://github.com/fsnotify/fsnotify/issues/11
71
+[#7]: https://github.com/howeyc/fsnotify/issues/7
72
+
44 73
 [contributing]: https://github.com/fsnotify/fsnotify/blob/master/CONTRIBUTING.md
45 74
 
46 75
 ## Related Projects
... ...
@@ -9,6 +9,7 @@ package fsnotify
9 9
 
10 10
 import (
11 11
 	"bytes"
12
+	"errors"
12 13
 	"fmt"
13 14
 )
14 15
 
... ...
@@ -60,3 +61,6 @@ func (op Op) String() string {
60 60
 func (e Event) String() string {
61 61
 	return fmt.Sprintf("%q: %s", e.Name, e.Op.String())
62 62
 }
63
+
64
+// Common errors that can be reported by a watcher
65
+var ErrEventOverflow = errors.New("fsnotify queue overflow")
... ...
@@ -24,7 +24,6 @@ type Watcher struct {
24 24
 	Events   chan Event
25 25
 	Errors   chan error
26 26
 	mu       sync.Mutex // Map access
27
-	cv       *sync.Cond // sync removing on rm_watch with IN_IGNORE
28 27
 	fd       int
29 28
 	poller   *fdPoller
30 29
 	watches  map[string]*watch // Map of inotify watches (key: path)
... ...
@@ -56,7 +55,6 @@ func NewWatcher() (*Watcher, error) {
56 56
 		done:     make(chan struct{}),
57 57
 		doneResp: make(chan struct{}),
58 58
 	}
59
-	w.cv = sync.NewCond(&w.mu)
60 59
 
61 60
 	go w.readEvents()
62 61
 	return w, nil
... ...
@@ -103,21 +101,23 @@ func (w *Watcher) Add(name string) error {
103 103
 	var flags uint32 = agnosticEvents
104 104
 
105 105
 	w.mu.Lock()
106
-	watchEntry, found := w.watches[name]
107
-	w.mu.Unlock()
108
-	if found {
109
-		watchEntry.flags |= flags
110
-		flags |= unix.IN_MASK_ADD
106
+	defer w.mu.Unlock()
107
+	watchEntry := w.watches[name]
108
+	if watchEntry != nil {
109
+		flags |= watchEntry.flags | unix.IN_MASK_ADD
111 110
 	}
112 111
 	wd, errno := unix.InotifyAddWatch(w.fd, name, flags)
113 112
 	if wd == -1 {
114 113
 		return errno
115 114
 	}
116 115
 
117
-	w.mu.Lock()
118
-	w.watches[name] = &watch{wd: uint32(wd), flags: flags}
119
-	w.paths[wd] = name
120
-	w.mu.Unlock()
116
+	if watchEntry == nil {
117
+		w.watches[name] = &watch{wd: uint32(wd), flags: flags}
118
+		w.paths[wd] = name
119
+	} else {
120
+		watchEntry.wd = uint32(wd)
121
+		watchEntry.flags = flags
122
+	}
121 123
 
122 124
 	return nil
123 125
 }
... ...
@@ -135,6 +135,13 @@ func (w *Watcher) Remove(name string) error {
135 135
 	if !ok {
136 136
 		return fmt.Errorf("can't remove non-existent inotify watch for: %s", name)
137 137
 	}
138
+
139
+	// We successfully removed the watch if InotifyRmWatch doesn't return an
140
+	// error, we need to clean up our internal state to ensure it matches
141
+	// inotify's kernel state.
142
+	delete(w.paths, int(watch.wd))
143
+	delete(w.watches, name)
144
+
138 145
 	// inotify_rm_watch will return EINVAL if the file has been deleted;
139 146
 	// the inotify will already have been removed.
140 147
 	// watches and pathes are deleted in ignoreLinux() implicitly and asynchronously
... ...
@@ -152,13 +159,6 @@ func (w *Watcher) Remove(name string) error {
152 152
 		return errno
153 153
 	}
154 154
 
155
-	// wait until ignoreLinux() deleting maps
156
-	exists := true
157
-	for exists {
158
-		w.cv.Wait()
159
-		_, exists = w.watches[name]
160
-	}
161
-
162 155
 	return nil
163 156
 }
164 157
 
... ...
@@ -245,13 +245,31 @@ func (w *Watcher) readEvents() {
245 245
 
246 246
 			mask := uint32(raw.Mask)
247 247
 			nameLen := uint32(raw.Len)
248
+
249
+			if mask&unix.IN_Q_OVERFLOW != 0 {
250
+				select {
251
+				case w.Errors <- ErrEventOverflow:
252
+				case <-w.done:
253
+					return
254
+				}
255
+			}
256
+
248 257
 			// If the event happened to the watched directory or the watched file, the kernel
249 258
 			// doesn't append the filename to the event, but we would like to always fill the
250 259
 			// the "Name" field with a valid filename. We retrieve the path of the watch from
251 260
 			// the "paths" map.
252 261
 			w.mu.Lock()
253
-			name := w.paths[int(raw.Wd)]
262
+			name, ok := w.paths[int(raw.Wd)]
263
+			// IN_DELETE_SELF occurs when the file/directory being watched is removed.
264
+			// This is a sign to clean up the maps, otherwise we are no longer in sync
265
+			// with the inotify kernel state which has already deleted the watch
266
+			// automatically.
267
+			if ok && mask&unix.IN_DELETE_SELF == unix.IN_DELETE_SELF {
268
+				delete(w.paths, int(raw.Wd))
269
+				delete(w.watches, name)
270
+			}
254 271
 			w.mu.Unlock()
272
+
255 273
 			if nameLen > 0 {
256 274
 				// Point "bytes" at the first byte of the filename
257 275
 				bytes := (*[unix.PathMax]byte)(unsafe.Pointer(&buf[offset+unix.SizeofInotifyEvent]))
... ...
@@ -262,7 +280,7 @@ func (w *Watcher) readEvents() {
262 262
 			event := newEvent(name, mask)
263 263
 
264 264
 			// Send the events that are not ignored on the events channel
265
-			if !event.ignoreLinux(w, raw.Wd, mask) {
265
+			if !event.ignoreLinux(mask) {
266 266
 				select {
267 267
 				case w.Events <- event:
268 268
 				case <-w.done:
... ...
@@ -279,15 +297,9 @@ func (w *Watcher) readEvents() {
279 279
 // Certain types of events can be "ignored" and not sent over the Events
280 280
 // channel. Such as events marked ignore by the kernel, or MODIFY events
281 281
 // against files that do not exist.
282
-func (e *Event) ignoreLinux(w *Watcher, wd int32, mask uint32) bool {
282
+func (e *Event) ignoreLinux(mask uint32) bool {
283 283
 	// Ignore anything the inotify API says to ignore
284 284
 	if mask&unix.IN_IGNORED == unix.IN_IGNORED {
285
-		w.mu.Lock()
286
-		defer w.mu.Unlock()
287
-		name := w.paths[int(wd)]
288
-		delete(w.paths, int(wd))
289
-		delete(w.watches, name)
290
-		w.cv.Broadcast()
291 285
 		return true
292 286
 	}
293 287