Skip to content

Commit 655fb07

Browse files
h9jianggopherbot
authored andcommitted
gopls/internal/filewatcher: synthesize events after watching a dir
Previously, file or subdirectory creations could be missed if they occurred after a directory was created but before the file watcher was registered for it. This resulted in missed events for all subsequent operations within that subdirectory. This change addresses this gap by performing a breadth-first scan of a new directory immediately after the watch is established. The scan iterates over the directory's entries, synthesizes `Create` events for each one. The BFS(pre-order) make sure the synthesized events follow logical order: the parent dir creation events always preceds those of their children. The root dir creation event and all synthesized `Create` are added to the out atomically to make sure the out slice is always holding a logically correct file events and ready to flush anytime. Note: file/dir creation events maybe duplicated, one captured by the scan, one captured by fsnotify but it is promised there is no miss. For golang/go#74292 Change-Id: Id9b63887d9a7fc83e608bf8390b2f7c198cfc160 Reviewed-on: https://go-review.googlesource.com/c/tools/+/716260 Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Hongxiang Jiang <hxjiang@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 0a420c7 commit 655fb07

File tree

2 files changed

+289
-145
lines changed

2 files changed

+289
-145
lines changed

gopls/internal/filewatcher/filewatcher.go

Lines changed: 61 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,49 @@ func (w *Watcher) process(errHandler func(error)) {
181181
continue
182182
}
183183

184+
var synthesized []protocol.FileEvent // synthesized create events
185+
184186
if isDir {
185187
switch e.Type {
186188
case protocol.Created:
187-
if err := w.watchDir(event.Name); err != nil {
188-
errHandler(err)
189-
}
189+
// Walks the entire directory tree, synthesizes create events for its contents,
190+
// and establishes watches for subdirectories. This recursive, pre-order
191+
// traversal using filepath.WalkDir guarantees a logical event sequence:
192+
// parent directory creation events always precede those of their children.
193+
//
194+
// For example, consider a creation event for directory a, and suppose
195+
// a has contents [a/b, a/b/c, a/c, a/c/d]. The effective events will be:
196+
//
197+
// CREATE a
198+
// CREATE a/b
199+
// CREATE a/b/c
200+
// CREATE a/c
201+
// CREATE a/c/d
202+
filepath.WalkDir(event.Name, func(path string, d fs.DirEntry, err error) error {
203+
if d.IsDir() && skipDir(d.Name()) {
204+
return filepath.SkipDir
205+
}
206+
if !d.IsDir() && skipFile(d.Name()) {
207+
return nil
208+
}
209+
210+
if path != event.Name { // avoid duplicate create event for root
211+
synthesized = append(synthesized, protocol.FileEvent{
212+
URI: protocol.URIFromPath(path),
213+
Type: protocol.Created,
214+
})
215+
}
216+
217+
if d.IsDir() {
218+
if err := w.watchDir(path); err != nil {
219+
errHandler(err)
220+
return filepath.SkipDir
221+
}
222+
}
223+
224+
return nil
225+
})
226+
190227
case protocol.Deleted:
191228
// Upon removal, we only need to remove the entries from
192229
// the map. The [fsnotify.Watcher] removes the watch for
@@ -200,6 +237,11 @@ func (w *Watcher) process(errHandler func(error)) {
200237
}
201238
}
202239

240+
// Discovered events must be appended to the 'out' slice atomically.
241+
// This ensures that at any point, the slice contains a logically
242+
// correct (maybe slightly outdated) batch of file events that is
243+
// ready to be flushed.
244+
w.mu.Lock()
203245
// Some systems emit duplicate change events in close
204246
// succession upon file modification. While the current
205247
// deduplication is naive and only handles immediate duplicates,
@@ -210,10 +252,10 @@ func (w *Watcher) process(errHandler func(error)) {
210252
// events means all duplicates, regardless of proximity, should
211253
// be removed. Consider checking the entire buffered slice or
212254
// using a map for this.
213-
w.mu.Lock()
214255
if len(w.out) == 0 || w.out[len(w.out)-1] != e {
215256
w.out = append(w.out, e)
216257
}
258+
w.out = append(w.out, synthesized...) // synthesized events are guaranteed to be unique
217259
w.mu.Unlock()
218260
}
219261
}
@@ -244,6 +286,16 @@ func skipDir(dirName string) bool {
244286
return strings.HasPrefix(dirName, ".") || strings.HasPrefix(dirName, "_") || dirName == "testdata"
245287
}
246288

289+
// skipFile reports whether the file should be skipped.
290+
func skipFile(fileName string) bool {
291+
switch strings.TrimPrefix(filepath.Ext(fileName), ".") {
292+
case "go", "mod", "sum", "work", "s":
293+
return false
294+
default:
295+
return true
296+
}
297+
}
298+
247299
// WatchDir walks through the directory and all its subdirectories, adding
248300
// them to the watcher.
249301
func (w *Watcher) WatchDir(path string) error {
@@ -282,16 +334,11 @@ func (w *Watcher) convertEvent(event fsnotify.Event) (_ protocol.FileEvent, isDi
282334
}
283335

284336
// Filter out events for directories and files that are not of interest.
285-
if isDir {
286-
if skipDir(filepath.Base(event.Name)) {
287-
return protocol.FileEvent{}, true
288-
}
289-
} else {
290-
switch strings.TrimPrefix(filepath.Ext(event.Name), ".") {
291-
case "go", "mod", "sum", "work", "s":
292-
default:
293-
return protocol.FileEvent{}, false
294-
}
337+
if isDir && skipDir(filepath.Base(event.Name)) {
338+
return protocol.FileEvent{}, true
339+
}
340+
if !isDir && skipFile(filepath.Base(event.Name)) {
341+
return protocol.FileEvent{}, false
295342
}
296343

297344
var t protocol.FileChangeType
@@ -338,15 +385,6 @@ func (w *Watcher) watchDir(path string) error {
338385
// links. This state can occur temporarily during operations like a git
339386
// branch switch. To handle this, we retry multiple times with exponential
340387
// backoff, allowing time for the symbolic link's target to be created.
341-
342-
// TODO(hxjiang): Address a race condition where file or directory creations
343-
// under current directory might be missed between the current directory
344-
// creation and the establishment of the file watch.
345-
//
346-
// To fix this, we should:
347-
// 1. Retrospectively check for and trigger creation events for any new
348-
// files/directories.
349-
// 2. Recursively add watches for any newly created subdirectories.
350388
var (
351389
delay = 500 * time.Millisecond
352390
err error

0 commit comments

Comments
 (0)