[tools] gopls/internal/filewatcher: add poll-based watcher

2 views
Skip to first unread message

Hongxiang Jiang (Gerrit)

unread,
Mar 11, 2026, 2:11:38 PM (8 days ago) Mar 11
to goph...@pubsubhelper.golang.org, Alan Donovan, golang-co...@googlegroups.com

Hongxiang Jiang has uploaded the change for review

Commit message

gopls/internal/filewatcher: add poll-based watcher

This change introduces a Watcher interface to allow for alternative
file-watching implementations (e.g., a git status-like scanner).
The existing fsnotify-based implementation is renamed to fsnotifyWatcher.

Also, update walkWatcher to support watching multiple root directories.
It also ensures that WatchDir performs a synchronous scan to establish a
baseline state if one doesn't exist, preventing missed events on startup.
Change-Id: Ieaac1cbc6ccc0ac1d0fc71dc9c48037a4dcf8a24

Change diff

diff --git a/gopls/internal/cmd/mcp.go b/gopls/internal/cmd/mcp.go
index d8f580d..1937711 100644
--- a/gopls/internal/cmd/mcp.go
+++ b/gopls/internal/cmd/mcp.go
@@ -109,7 +109,8 @@
errHandler := func(err error) {
log.Printf("watch error: %v", err)
}
- w, err := filewatcher.New(500*time.Millisecond, nil, func(events []protocol.FileEvent) {
+ // FIXME: respect settings. Use an enum.
+ w, err := filewatcher.New("fsnotify", 500*time.Millisecond, nil, func(events []protocol.FileEvent) {
if len(events) == 0 {
return
}
diff --git a/gopls/internal/filewatcher/filewatcher.go b/gopls/internal/filewatcher/filewatcher.go
index ad7519e..5d40d2b 100644
--- a/gopls/internal/filewatcher/filewatcher.go
+++ b/gopls/internal/filewatcher/filewatcher.go
@@ -1,51 +1,28 @@
-// Copyright 2025 The Go Authors. All rights reserved.
+// Copyright 2026 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package filewatcher

import (
- "errors"
- "io/fs"
+ "fmt"
"log/slog"
- "os"
- "path/filepath"
- "strings"
- "sync"
"time"

- "github.com/fsnotify/fsnotify"
"golang.org/x/tools/gopls/internal/protocol"
- "golang.org/x/tools/internal/robustio"
)

-// ErrClosed is used when trying to operate on a closed Watcher.
-var ErrClosed = errors.New("file watcher: watcher already closed")
+// Watcher monitors file system events.
+type Watcher interface {
+ // WatchDir adds a directory to the set of watched directories.
+ WatchDir(path string) error

-// Watcher collects events from a [fsnotify.Watcher] and converts them into
-// batched LSP [protocol.FileEvent]s.
-type Watcher struct {
- logger *slog.Logger
+ // Close stops the watcher and releases any associated resources.
+ Close() error

- stop chan struct{} // closed by Close to terminate run and process loop
- wg sync.WaitGroup // counts the number of active run and process goroutines (max 2)
-
- ready chan struct{} // signals work to process
-
- watcher *fsnotify.Watcher
-
- mu sync.Mutex // guards all fields below
-
- // in is the queue of fsnotify events waiting to be processed.
- in []fsnotify.Event
-
- // out is the current batch of unsent file events, which will be sent when
- // the timer expires.
- out []protocol.FileEvent
-
- // knownDirs tracks all known directories to help distinguish between file
- // and directory deletion events.
- knownDirs map[string]struct{}
+ // Poke signals the watcher to prioritize a scan, if applicable.
+ // This is used to implement adaptive polling.
+ Poke()
}

// New creates a new file watcher and starts its event-handling loop. The
@@ -54,486 +31,15 @@
// The provided event handler is called sequentially with a batch of file events,
// but the error handler is called concurrently. The watcher blocks until the
// handler returns, so the handlers should be fast and non-blocking.
-func New(delay time.Duration, logger *slog.Logger, eventsHandler func([]protocol.FileEvent), errHandler func(error)) (*Watcher, error) {
- watcher, err := fsnotify.NewWatcher()
- if err != nil {
- return nil, err
+func New(mode string, interval time.Duration, logger *slog.Logger, onEvents func([]protocol.FileEvent), onError func(error)) (Watcher, error) {
+ switch mode {
+ case "poll":
+ return NewPollWatcher(interval, logger, onEvents, onError), nil
+ case "fsnotify":
+ return NewFSNotifyWatcher(interval, logger, onEvents, onError)
}
- w := &Watcher{
- logger: logger,
- watcher: watcher,
- knownDirs: make(map[string]struct{}),
- stop: make(chan struct{}),
- ready: make(chan struct{}, 1),
- }
-
- w.wg.Add(1)
- go w.run(eventsHandler, errHandler, delay)
-
- w.wg.Add(1)
- go w.process(errHandler)
-
- return w, nil
+ // TODO(hxjiang): support "auto" mode.
+ return nil, fmt.Errorf("unknown FileWatcher mode: %q", mode)
}

-// run is the receiver and sender loop.
-//
-// As receiver, its primary responsibility is to drain events and errors from
-// the fsnotify watcher as quickly as possible and enqueue events for processing
-// by the process goroutine. This is critical to work around a potential
-// fsnotify deadlock (see fsnotify/fsnotify#502).
-//
-// As sender, it manages a timer and flush events to the handler if there is
-// no events captured for a period of time.
-func (w *Watcher) run(eventsHandler func([]protocol.FileEvent), errHandler func(error), delay time.Duration) {
- defer w.wg.Done()
-
- timer := time.NewTimer(delay)
- defer timer.Stop()
-
- for {
- select {
- case <-w.stop:
- return
-
- case <-timer.C:
- // TODO(hxjiang): flush is triggered when there is no events captured
- // in a certain period of time, it may be better to flush it when the
- // w.in is completely empty.
- //
- // Currently, partial events may be emitted if a directory watch gets
- // stuck. While this does not affect correctness, it means events
- // might be sent to the client in multiple portions rather than a
- // single batch.
- w.mu.Lock()
- events := w.out
- w.out = nil
- w.mu.Unlock()
-
- if len(events) > 0 {
- eventsHandler(events)
- }
-
- timer.Reset(delay)
-
- case event, ok := <-w.watcher.Events:
- // The watcher closed. Continue the loop and let the <-w.stop case
- // handle the actual shutdown.
- if !ok {
- continue
- }
-
- // TODO(hxjiang): perform some filtering before we reset the timer
- // to avoid consistenly resetting the timer in a noisy file syestem,
- // or simply convert the event here.
- timer.Reset(delay)
-
- w.mu.Lock()
- w.in = append(w.in, event)
- w.mu.Unlock()
-
- w.signal()
-
- case err, ok := <-w.watcher.Errors:
- // The watcher closed. Continue the loop and let the <-w.stop case
- // handle the actual shutdown.
- if !ok {
- continue
- }
-
- errHandler(err)
- }
- }
-}
-
-// process is a worker goroutine that converts raw fsnotify events from queue
-// and handles the potentially blocking work of watching new directories. It is
-// the counterpart to the run goroutine.
-func (w *Watcher) process(errHandler func(error)) {
- defer w.wg.Done()
-
- for {
- select {
- case <-w.stop:
- return
-
- case <-w.ready:
- w.mu.Lock()
- events := w.in
- w.in = nil
- w.mu.Unlock()
-
- for _, event := range events {
- // File watcher is closing, drop any remaining work.
- select {
- case <-w.stop:
- return
- default:
- }
-
- // fsnotify does not guarantee clean filepaths.
- event.Name = filepath.Clean(event.Name)
-
- // fsnotify.Event should not be handled concurrently, to preserve their
- // original order. For example, if a file is deleted and recreated,
- // concurrent handling could process the events in reverse order.
- e, isDir := w.convertEvent(event)
- if e == (protocol.FileEvent{}) {
- continue
- }
-
- var synthesized []protocol.FileEvent // synthesized create events
-
- if isDir {
- switch e.Type {
- case protocol.Created:
- // Walks the entire directory tree, synthesizes create
- // events for its contents, and establishes watches for
- // subdirectories. This recursive, pre-order traversal
- // guarantees a logical event sequence: parent directory
- // creation events always precede those of their children.
- //
- // For example, consider a creation event for directory
- // a, and suppose a has contents [a/b, a/b/c, a/c, a/c/d].
- // The effective events will be:
- //
- // CREATE a
- // CREATE a/b
- // CREATE a/b/c
- // CREATE a/c
- // CREATE a/c/d
- w.walkDirWithRetry(event.Name, errHandler, func(path string, isDir bool) error {
- if path != event.Name {
- synthesized = append(synthesized, protocol.FileEvent{
- URI: protocol.URIFromPath(path),
- Type: protocol.Created,
- })
- }
-
- if isDir {
- return w.watchDir(path)
- } else {
- return nil
- }
- })
-
- case protocol.Deleted:
- // Upon removal, we only need to remove the entries from
- // the map. The [fsnotify.Watcher] removes the watch for
- // us. fsnotify/fsnotify#268
- w.mu.Lock()
- delete(w.knownDirs, event.Name)
- w.mu.Unlock()
- default:
- // convertEvent enforces that dirs are only Created or Deleted.
- panic("impossible")
- }
- }
-
- // Discovered events must be appended to the 'out' slice atomically.
- // This ensures that at any point, the slice contains a logically
- // correct (maybe slightly outdated) batch of file events that is
- // ready to be flushed.
- w.mu.Lock()
- // Some systems emit duplicate change events in close
- // succession upon file modification. While the current
- // deduplication is naive and only handles immediate duplicates,
- // a more robust solution is needed.
- // https://github.com/fsnotify/fsnotify?tab=readme-ov-file#why-do-i-get-many-chmod-events
- //
- // TODO(hxjiang): Enhance deduplication. The current batching of
- // events means all duplicates, regardless of proximity, should
- // be removed. Consider checking the entire buffered slice or
- // using a map for this.
- if len(w.out) == 0 || w.out[len(w.out)-1] != e {
- w.out = append(w.out, e)
- }
- w.out = append(w.out, synthesized...) // synthesized events are guaranteed to be unique
- w.mu.Unlock()
- }
- }
- }
-}
-
-// signal notifies the process goroutine that events are added to the queue and
-// ready for handling.
-func (w *Watcher) signal() {
- select {
- case w.ready <- struct{}{}:
- default:
- }
-}
-
-// skipDir reports whether the input dir should be skipped.
-// Directories that are unlikely to contain Go source files relevant for
-// analysis, such as .git directories or testdata, should be skipped to
-// avoid unnecessary file system notifications. This reduces noise and
-// improves efficiency. Conversely, any directory that might contain Go
-// source code should be watched to ensure that gopls can respond to
-// file changes.
-func skipDir(dirName string) bool {
- // TODO(hxjiang): the file watcher should honor gopls directory
- // filter or the new go.mod ignore directive, or actively listening
- // to gopls register capability request with method
- // "workspace/didChangeWatchedFiles" like a real LSP client.
- return strings.HasPrefix(dirName, ".") || strings.HasPrefix(dirName, "_") || dirName == "testdata"
-}
-
-// skipFile reports whether the file should be skipped.
-func skipFile(fileName string) bool {
- switch strings.TrimPrefix(filepath.Ext(fileName), ".") {
- case "go", "mod", "sum", "work", "s":
- return false
- default:
- return true
- }
-}
-
-// WatchDir walks through the directory and all its subdirectories, adding
-// them to the watcher.
-func (w *Watcher) WatchDir(path string) error {
- return filepath.WalkDir(filepath.Clean(path), func(path string, d fs.DirEntry, err error) error {
- if d.IsDir() {
- if skipDir(d.Name()) {
- return filepath.SkipDir
- }
-
- return w.watchDir(path)
- }
- return nil
- })
-}
-
-// convertEvent translates an [fsnotify.Event] into a [protocol.FileEvent].
-// It returns the translated event and a boolean indicating if the path was a
-// directory. For directories, the event Type is either Created or Deleted.
-// It returns empty event for events that should be ignored.
-func (w *Watcher) convertEvent(event fsnotify.Event) (_ protocol.FileEvent, isDir bool) {
- // Determine if the event is for a directory.
- if info, err := os.Stat(event.Name); err == nil {
- isDir = info.IsDir()
- } else if os.IsNotExist(err) {
- // Upon deletion, the file/dir has been removed. fsnotify does not
- // provide information regarding the deleted item.
- // Use the set of known directories to determine if the deleted item was a directory.
- isDir = w.isWatchedDir(event.Name)
- } else {
- // If statting failed, something is wrong with the file system.
- // Log and move on.
- if w.logger != nil {
- w.logger.Error("failed to stat path, skipping event as its type (file/dir) is unknown", "path", event.Name, "err", err)
- }
- return protocol.FileEvent{}, false
- }
-
- // Filter out events for directories and files that are not of interest.
- if isDir && skipDir(filepath.Base(event.Name)) {
- return protocol.FileEvent{}, true
- }
- if !isDir && skipFile(filepath.Base(event.Name)) {
- return protocol.FileEvent{}, false
- }
-
- var t protocol.FileChangeType
- switch {
- case event.Op.Has(fsnotify.Rename):
- // A rename is treated as a deletion of the old path because the
- // fsnotify RENAME event doesn't include the new path. A separate
- // CREATE event will be sent for the new path if the destination
- // directory is watched.
- fallthrough
- case event.Op.Has(fsnotify.Remove):
- // TODO(hxjiang): Directory removal events from some LSP clients may
- // not include corresponding removal events for child files and
- // subdirectories. Should we do some filtering when adding the dir
- // deletion event to the events slice.
- t = protocol.Deleted
- case event.Op.Has(fsnotify.Create):
- t = protocol.Created
- case event.Op.Has(fsnotify.Write):
- if isDir {
- return protocol.FileEvent{}, isDir // ignore dir write events
- }
- t = protocol.Changed
- default:
- return protocol.FileEvent{}, isDir // ignore the rest of the events
- }
-
- return protocol.FileEvent{
- URI: protocol.URIFromPath(event.Name),
- Type: t,
- }, isDir
-}
-
-// watchDir registers a watch for a directory, retrying with backoff if it fails.
-//
-// Returns nil on success or watcher closing; otherwise, the last error after
-// all retries.
-func (w *Watcher) watchDir(path string) error {
- w.mu.Lock()
- w.knownDirs[path] = struct{}{}
- w.mu.Unlock()
-
- // On darwin, watching a directory will fail if it contains broken symbolic
- // links. This state can occur temporarily during operations like a git
- // branch switch. To handle this, we retry multiple times with exponential
- // backoff, allowing time for the symbolic link's target to be created.
- var (
- delay = 500 * time.Millisecond
- err error
- )
-
- for i := range 5 {
- if i > 0 {
- select {
- case <-time.After(delay):
- delay *= 2
- case <-w.stop:
- return nil
- }
- }
- // This function may block due to fsnotify/fsnotify#502.
- err = w.watcher.Add(path)
- if afterAddHook != nil {
- afterAddHook(path, err)
- }
- if err == nil {
- break
- }
- }
-
- return err
-}
-
-var afterAddHook func(path string, err error)
-
-// isWatchedDir reports whether the given path is a known directory that
-// the watcher is managing.
-func (w *Watcher) isWatchedDir(path string) bool {
- w.mu.Lock()
- defer w.mu.Unlock()
-
- _, isDir := w.knownDirs[path]
- return isDir
-}
-
-// Close shuts down the watcher, waits for the internal goroutine to terminate,
-// and returns any final error.
-func (w *Watcher) Close() error {
- // Wait for fsnotify' watcher to terminate.
- err := w.watcher.Close()
-
- // Wait for run and process loop to terminate. It's important to stop the
- // run and process loop the last place because we don't know whether
- // fsnotify's watcher expect us to keep consuming events or errors from
- // [fsnotify.Watcher.Events] and [fsnotify.Watcher.Errors] while it's being
- // closed.
- // To avoid any potential deadlock, have the channel receiver running until
- // the last minute.
- close(w.stop)
- w.wg.Wait()
-
- return err
-}
-
-// walkDir calls fn against current path and recursively descends path for each
-// file or directory of our interest.
-func (w *Watcher) walkDir(path string, isDir bool, errHandler func(error), fn func(path string, isDir bool) error) {
- if err := fn(path, isDir); err != nil {
- errHandler(err)
- return
- }
-
- if !isDir {
- return
- }
-
- entries, err := tryFSOperation(w.stop, func() ([]fs.DirEntry, error) {
- // ReadDir may fail due because other processes may be actively
- // modifying the watched dir see golang/go#74820.
- // TODO(hxjiang): consider adding robustio.ReadDir.
- return os.ReadDir(path)
- })
- if err != nil {
- if err != ErrClosed {
- errHandler(err)
- }
- return
- }
-
- for _, e := range entries {
- if e.IsDir() && skipDir(e.Name()) {
- continue
- }
- if !e.IsDir() && skipFile(e.Name()) {
- continue
- }
-
- w.walkDir(filepath.Join(path, e.Name()), e.IsDir(), errHandler, fn)
- }
-}
-
-// walkDirWithRetry walks the file tree rooted at root, calling fn for each
-// file or directory of our interest in the tree, including root.
-//
-// All errors that arise visiting directories or files will be reported to the
-// provided error handler function. If an error is encountered visiting a
-// directory, that entire subtree will be skipped.
-//
-// walkDirWithRetry does not follow symbolic links.
-//
-// It is used instead of [filepath.WalkDir] because it provides control over
-// retry behavior when reading a directory fails. If [os.ReadDir] fails with an
-// ephemeral error, it is retried multiple times with exponential backoff.
-//
-// TODO(hxjiang): call walkDirWithRetry in WalkDir.
-func (w *Watcher) walkDirWithRetry(root string, errHandler func(error), fn func(path string, isDir bool) error) {
- info, err := tryFSOperation(w.stop, func() (os.FileInfo, error) {
- return os.Lstat(root) // [os.Lstat] does not follow symlink.
- })
- if err != nil {
- if err != ErrClosed {
- errHandler(err)
- }
- return
- }
-
- w.walkDir(root, info.IsDir(), errHandler, fn)
-}
-
-// tryFSOperation executes a function `op` with retry logic, making it resilient
-// to transient errors. It attempts the operation up to 5 times with exponential
-// backoff. Retries occur only if the error is ephemeral.
-//
-// The operation can be interrupted by closing the `stop` channel, in which case
-// it returns [ErrClosed].
-func tryFSOperation[Result any](stop <-chan struct{}, op func() (Result, error)) (Result, error) {
- var (
- delay = 50 * time.Millisecond
- err error
- )
-
- for i := range 5 {
- if i > 0 {
- select {
- case <-time.After(delay):
- delay *= 2
- case <-stop:
- var zero Result
- return zero, ErrClosed
- }
- }
-
- var res Result
- res, err = op()
-
- if robustio.IsEphemeralError(err) {
- continue
- } else {
- return res, err
- }
- }
-
- var zero Result
- return zero, err // return last error encountered
-}
+type Unit = struct{}
diff --git a/gopls/internal/filewatcher/filewatcher_test.go b/gopls/internal/filewatcher/filewatcher_test.go
index 7084d86..3665ed1 100644
--- a/gopls/internal/filewatcher/filewatcher_test.go
+++ b/gopls/internal/filewatcher/filewatcher_test.go
@@ -23,12 +23,6 @@
)

func TestFileWatcher(t *testing.T) {
- switch runtime.GOOS {
- case "darwin", "linux", "windows":
- default:
- t.Skip("unsupported OS")
- }
-
testCases := []struct {
name string
goos []string // if not empty, only run in these OS.
@@ -194,90 +188,166 @@

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
- if len(tt.goos) > 0 && !slices.Contains(tt.goos, runtime.GOOS) {
- t.Skipf("skipping on %s", runtime.GOOS)
- }
-
- root := t.TempDir()
-
- archive := txtar.Parse([]byte(tt.initWorkspace))
- for _, f := range archive.Files {
- path := filepath.Join(root, f.Name)
- if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
- t.Fatal(err)
- }
- if err := os.WriteFile(path, f.Data, 0644); err != nil {
- t.Fatal(err)
- }
- }
-
- foundAll := make(chan struct{})
- var gots []protocol.FileEvent
-
- matched := 0
- eventsHandler := func(events []protocol.FileEvent) {
- gots = append(gots, events...)
-
- if matched == len(tt.expectedEvents) {
- return
- }
-
- // This verifies that the list of wanted events is a subsequence of
- // the received events. It confirms not only that all wanted events
- // are present, but also that their relative order is preserved.
- for _, got := range events {
- want := protocol.FileEvent{
- URI: protocol.URIFromPath(filepath.Join(root, string(tt.expectedEvents[matched].URI))),
- Type: tt.expectedEvents[matched].Type,
+ for _, mode := range []string{"fsnotify", "poll"} {
+ t.Run(mode, func(t *testing.T) {
+ if mode == "fsnotify" {
+ if len(tt.goos) > 0 && !slices.Contains(tt.goos, runtime.GOOS) {
+ t.Skipf("skipping on %s", runtime.GOOS)
+ }
}
- if want == got {
- matched++
+
+ root := t.TempDir()
+
+ archive := txtar.Parse([]byte(tt.initWorkspace))
+ for _, f := range archive.Files {
+ path := filepath.Join(root, f.Name)
+ if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(path, f.Data, 0644); err != nil {
+ t.Fatal(err)
+ }
}
- if matched == len(tt.expectedEvents) {
- close(foundAll)
- return
+
+ foundAll := make(chan filewatcher.Unit)
+ var gots []protocol.FileEvent
+
+ matched := 0
+ eventsHandler := func(events []protocol.FileEvent) {
+ gots = append(gots, events...)
+
+ if matched == len(tt.expectedEvents) {
+ return
+ }
+
+ // The "poll" watcher might coalesce "Created" and "Changed"
+ // events if it scans after both have occurred.
+ // Adjust the expected events for this test case.
+ expected := tt.expectedEvents
+ if mode == "poll" {
+ var filtered []protocol.FileEvent
+ for i, e := range expected {
+ // Skip "Changed" if preceded by "Created" for the same URI.
+ if e.Type == protocol.Changed && i > 0 && expected[i-1].Type == protocol.Created && expected[i-1].URI == e.URI {
+ continue
+ }
+ filtered = append(filtered, e)
+ }
+ expected = filtered
+ }
+
+ // If mode is "poll", the order of "Created" and "Deleted" events
+ // during a rename is not guaranteed to match fsnotify.
+ // We check if all expected events are present in the 'gots' slice.
+ if mode == "poll" {
+ allMatched := true
+ for _, w := range expected {
+ want := protocol.FileEvent{
+ URI: protocol.URIFromPath(filepath.Join(root, string(w.URI))),
+ Type: w.Type,
+ }
+ found := false
+ for _, g := range gots {
+ if g == want {
+ found = true
+ break
+ }
+ }
+ if !found {
+ allMatched = false
+ break
+ }
+ }
+ if allMatched {
+ select {
+ case <-foundAll:
+ default:
+ close(foundAll)
+ }
+ }
+ return
+ }
+
+ // This verifies that the list of wanted events is a subsequence of
+ // the received events. It confirms not only that all wanted events
+ // are present, but also that their relative order is preserved.
+ for _, got := range events {
+ if matched == len(expected) {
+ break
+ }
+ want := protocol.FileEvent{
+ URI: protocol.URIFromPath(filepath.Join(root, string(expected[matched].URI))),
+ Type: expected[matched].Type,
+ }
+ if want == got {
+ matched++
+ }
+ }
+
+ if matched == len(expected) {
+ select {
+ case <-foundAll:
+ default:
+ close(foundAll)
+ }
+ return
+ }
+
}
- }
-
- }
- errHandler := func(err error) {
- t.Errorf("error from watcher: %v", err)
- }
- w, err := filewatcher.New(50*time.Millisecond, nil, eventsHandler, errHandler)
- if err != nil {
- t.Fatal(err)
- }
- defer func() {
- if err := w.Close(); err != nil {
- t.Errorf("failed to close the file watcher: %v", err)
- }
- }()
-
- if err := w.WatchDir(root); err != nil {
- t.Fatal(err)
- }
-
- if tt.changes != nil {
- if err := tt.changes(root); err != nil {
- t.Fatal(err)
- }
- }
-
- select {
- case <-foundAll:
- case <-time.After(30 * time.Second):
- if matched != len(tt.expectedEvents) {
- var want strings.Builder
- for _, e := range tt.expectedEvents {
- want.WriteString(fmt.Sprintf("URI: %s type: %v\n", e.URI, e.Type))
+ errHandler := func(err error) {
+ t.Errorf("error from watcher: %v", err)
}
- var got strings.Builder
- for _, e := range gots {
- got.WriteString(fmt.Sprintf("URI: %s type: %v\n", strings.TrimPrefix(e.URI.Path(), root+"/"), e.Type))
- }
- t.Errorf("found %v matching events slice\nwant sequences:\n%s\nall got:\n%s", matched, want.String(), got.String())
- }

+ // Use shorter delay for poll watcher to make test faster.
+ delay := 50 * time.Millisecond
+ if mode == "poll" {
+ delay = 10 * time.Millisecond
+ }
+
+ w, err := filewatcher.New(mode, delay, nil, eventsHandler, errHandler)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer func() {
+ if err := w.Close(); err != nil {
+ t.Errorf("failed to close the file watcher: %v", err)
+ }
+ }()
+
+ if err := w.WatchDir(root); err != nil {
+ t.Fatal(err)
+ }
+
+ if tt.changes != nil {
+ // For poll watcher, we need to ensure mtime changes.
+ // The test cases use os.WriteFile which updates mtime.
+ // However, if the test runs too fast, mtime might not change (if resolution is low).
+ // Poll watcher checks mtime.
+ if mode == "poll" {
+ time.Sleep(10 * time.Millisecond)
+ }
+ if err := tt.changes(root); err != nil {
+ t.Fatal(err)
+ }
+ }
+
+ select {
+ case <-foundAll:
+ case <-time.After(30 * time.Second):
+ if matched != len(tt.expectedEvents) {
+ var want strings.Builder
+ for _, e := range tt.expectedEvents {
+ want.WriteString(fmt.Sprintf("URI: %s type: %v\n", e.URI, e.Type))
+ }
+ var got strings.Builder
+ for _, e := range gots {
+ got.WriteString(fmt.Sprintf("URI: %s type: %v\n", strings.TrimPrefix(e.URI.Path(), root+"/"), e.Type))
+ }
+ t.Errorf("found %v matching events slice\nwant sequences:\n%s\nall got:\n%s", matched, want.String(), got.String())
+ }
+
+ }
+ })
}
})
}
@@ -312,7 +382,7 @@
var (
gots []protocol.FileEvent
matched int
- foundAll = make(chan struct{})
+ foundAll = make(chan filewatcher.Unit)
)
wants := []protocol.FileEvent{
// "foo" create event from fsnotify and synthesized create event
@@ -351,7 +421,7 @@
errHandler := func(err error) {
t.Errorf("error from watcher: %v", err)
}
- w, err := filewatcher.New(50*time.Millisecond, nil, eventsHandler, errHandler)
+ w, err := filewatcher.New("fsnotify", 50*time.Millisecond, nil, eventsHandler, errHandler)
if err != nil {
t.Fatal(err)
}
@@ -466,124 +536,136 @@
}

func TestStress(t *testing.T) {
- switch runtime.GOOS {
- case "darwin", "linux", "windows":
- default:
- t.Skip("unsupported OS")
- }
+ for _, mode := range []string{"fsnotify", "poll"} {
+ t.Run(mode, func(t *testing.T) {
+ if mode == "poll" {
+ t.Skip("stress test is inherently racy for polling watcher")
+ }
+ if mode == "fsnotify" {
+ switch runtime.GOOS {
+ case "darwin", "linux", "windows":
+ default:
+ t.Skip("unsupported OS")
+ }
+ }

- const (
- delay = 50 * time.Millisecond
- parallelism = 100 // number of parallel instances of each kind of operation
- )
+ const (
+ parallelism = 100 // number of parallel instances of each kind of operation
+ )
+ delay := 50 * time.Millisecond
+ if mode == "poll" {
+ delay = 10 * time.Millisecond
+ }

- root := t.TempDir()
+ root := t.TempDir()

- mkdir := func(base string) func() error {
- return func() error {
- return os.Mkdir(filepath.Join(root, base), 0755)
- }
- }
- write := func(base string) func() error {
- return func() error {
- return os.WriteFile(filepath.Join(root, base), []byte("package main"), 0644)
- }
- }
- remove := func(base string) func() error {
- return func() error {
- return os.Remove(filepath.Join(root, base))
- }
- }
- rename := func(old, new string) func() error {
- return func() error {
- return os.Rename(filepath.Join(root, old), filepath.Join(root, new))
- }
- }
+ mkdir := func(base string) func() error {
+ return func() error {
+ return os.Mkdir(filepath.Join(root, base), 0755)
+ }
+ }
+ write := func(base string) func() error {
+ return func() error {
+ return os.WriteFile(filepath.Join(root, base), []byte("package main"), 0644)
+ }
+ }
+ remove := func(base string) func() error {
+ return func() error {
+ return os.Remove(filepath.Join(root, base))
+ }
+ }
+ rename := func(old, new string) func() error {
+ return func() error {
+ return os.Rename(filepath.Join(root, old), filepath.Join(root, new))
+ }
+ }

- wants := make(map[protocol.FileEvent]bool)
- want := func(base string, t protocol.FileChangeType) {
- wants[protocol.FileEvent{URI: protocol.URIFromPath(filepath.Join(root, base)), Type: t}] = true
- }
+ wants := make(map[protocol.FileEvent]bool)
+ want := func(base string, t protocol.FileChangeType) {
+ wants[protocol.FileEvent{URI: protocol.URIFromPath(filepath.Join(root, base)), Type: t}] = true
+ }

- for i := range parallelism {
- // Create files and dirs that will be deleted or renamed later.
- if err := cmp.Or(
- mkdir(fmt.Sprintf("delete-dir-%d", i))(),
- mkdir(fmt.Sprintf("old-dir-%d", i))(),
- write(fmt.Sprintf("delete-file-%d.go", i))(),
- write(fmt.Sprintf("old-file-%d.go", i))(),
- ); err != nil {
- t.Fatal(err)
- }
+ for i := range parallelism {
+ // Create files and dirs that will be deleted or renamed later.
+ if err := cmp.Or(
+ mkdir(fmt.Sprintf("delete-dir-%d", i))(),
+ mkdir(fmt.Sprintf("old-dir-%d", i))(),
+ write(fmt.Sprintf("delete-file-%d.go", i))(),
+ write(fmt.Sprintf("old-file-%d.go", i))(),
+ ); err != nil {
+ t.Fatal(err)
+ }

- // Add expected notification events to the "wants" set.
- want(fmt.Sprintf("file-%d.go", i), protocol.Created)
- want(fmt.Sprintf("delete-file-%d.go", i), protocol.Deleted)
- want(fmt.Sprintf("old-file-%d.go", i), protocol.Deleted)
- want(fmt.Sprintf("new-file-%d.go", i), protocol.Created)
- want(fmt.Sprintf("dir-%d", i), protocol.Created)
- want(fmt.Sprintf("delete-dir-%d", i), protocol.Deleted)
- want(fmt.Sprintf("old-dir-%d", i), protocol.Deleted)
- want(fmt.Sprintf("new-dir-%d", i), protocol.Created)
- }
+ // Add expected notification events to the "wants" set.
+ want(fmt.Sprintf("file-%d.go", i), protocol.Created)
+ want(fmt.Sprintf("delete-file-%d.go", i), protocol.Deleted)
+ want(fmt.Sprintf("old-file-%d.go", i), protocol.Deleted)
+ want(fmt.Sprintf("new-file-%d.go", i), protocol.Created)
+ want(fmt.Sprintf("dir-%d", i), protocol.Created)
+ want(fmt.Sprintf("delete-dir-%d", i), protocol.Deleted)
+ want(fmt.Sprintf("old-dir-%d", i), protocol.Deleted)
+ want(fmt.Sprintf("new-dir-%d", i), protocol.Created)
+ }

- foundAll := make(chan struct{})
+ foundAll := make(chan filewatcher.Unit)

- eventsHandler := func(events []protocol.FileEvent) {
- if len(wants) == 0 { // avoid closing twice
- return
- }
- for _, e := range events {
- delete(wants, e)
- }
- if len(wants) == 0 {
- close(foundAll)
- }
- }
- errHandler := func(err error) {
- t.Errorf("error from watcher: %v", err)
- }
- w, err := filewatcher.New(delay, nil, eventsHandler, errHandler)
- if err != nil {
- t.Fatal(err)
- }
- defer func() {
- if err := w.Close(); err != nil {
- t.Errorf("failed to close the file watcher: %v", err)
- }
- }()
+ eventsHandler := func(events []protocol.FileEvent) {
+ if len(wants) == 0 { // avoid closing twice
+ return
+ }
+ for _, e := range events {
+ delete(wants, e)
+ }
+ if len(wants) == 0 {
+ close(foundAll)
+ }
+ }
+ errHandler := func(err error) {
+ t.Errorf("error from watcher: %v", err)
+ }
+ w, err := filewatcher.New(mode, delay, nil, eventsHandler, errHandler)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer func() {
+ if err := w.Close(); err != nil {
+ t.Errorf("failed to close the file watcher: %v", err)
+ }
+ }()

- if err := w.WatchDir(root); err != nil {
- t.Fatal(err)
- }
+ if err := w.WatchDir(root); err != nil {
+ t.Fatal(err)
+ }

- // Spin up multiple goroutines, to perform 6 file system operations i.e.
- // create, delete, rename of file or directory. For deletion and rename,
- // the goroutine deletes / renames files or directories created before the
- // watcher starts.
- var g errgroup.Group
- for id := range parallelism {
- ops := []func() error{
- write(fmt.Sprintf("file-%d.go", id)),
- remove(fmt.Sprintf("delete-file-%d.go", id)),
- rename(fmt.Sprintf("old-file-%d.go", id), fmt.Sprintf("new-file-%d.go", id)),
- mkdir(fmt.Sprintf("dir-%d", id)),
- remove(fmt.Sprintf("delete-dir-%d", id)),
- rename(fmt.Sprintf("old-dir-%d", id), fmt.Sprintf("new-dir-%d", id)),
- }
- for _, f := range ops {
- g.Go(f)
- }
- }
- if err := g.Wait(); err != nil {
- t.Fatal(err)
- }
+ // Spin up multiple goroutines, to perform 6 file system operations i.e.
+ // create, delete, rename of file or directory. For deletion and rename,
+ // the goroutine deletes / renames files or directories created before the
+ // watcher starts.
+ var g errgroup.Group
+ for id := range parallelism {
+ ops := []func() error{
+ write(fmt.Sprintf("file-%d.go", id)),
+ remove(fmt.Sprintf("delete-file-%d.go", id)),
+ rename(fmt.Sprintf("old-file-%d.go", id), fmt.Sprintf("new-file-%d.go", id)),
+ mkdir(fmt.Sprintf("dir-%d", id)),
+ remove(fmt.Sprintf("delete-dir-%d", id)),
+ rename(fmt.Sprintf("old-dir-%d", id), fmt.Sprintf("new-dir-%d", id)),
+ }
+ for _, f := range ops {
+ g.Go(f)
+ }
+ }
+ if err := g.Wait(); err != nil {
+ t.Fatal(err)
+ }

- select {
- case <-foundAll:
- case <-time.After(30 * time.Second):
- if len(wants) > 0 {
- t.Errorf("missing expected events: %#v", moremaps.KeySlice(wants))
- }
+ select {
+ case <-foundAll:
+ case <-time.After(30 * time.Second):
+ if len(wants) > 0 {
+ t.Errorf("missing expected events: %#v", moremaps.KeySlice(wants))
+ }
+ }
+ })
}
}
diff --git a/gopls/internal/filewatcher/fsnotify_watcher.go b/gopls/internal/filewatcher/fsnotify_watcher.go
new file mode 100644
index 0000000..f3e84d1
--- /dev/null
+++ b/gopls/internal/filewatcher/fsnotify_watcher.go
@@ -0,0 +1,533 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package filewatcher
+
+import (
+ "errors"
+ "io/fs"
+ "log/slog"
+ "os"
+ "path/filepath"
+ "strings"
+ "sync"
+ "time"
+
+ "github.com/fsnotify/fsnotify"
+ "golang.org/x/tools/gopls/internal/protocol"
+ "golang.org/x/tools/internal/robustio"
+)
+
+// ErrClosed is used when trying to operate on a closed Watcher.
+var ErrClosed = errors.New("file watcher: watcher already closed")
+
+func NewFSNotifyWatcher(interval time.Duration, logger *slog.Logger, onEvents func([]protocol.FileEvent), onError func(error)) (Watcher, error) {
+ watcher, err := fsnotify.NewWatcher()
+ if err != nil {
+ return nil, err
+ }
+ w := &fsnotifyWatcher{
+ logger: logger,
+ watcher: watcher,
+ interval: interval,
+ onEvents: onEvents,
+ onError: onError,
+ knownDirs: make(map[string]Unit),
+ stop: make(chan Unit),
+ ready: make(chan Unit, 1),
+ }
+ w.wg.Go(w.run)
+ w.wg.Go(w.process)
+ return w, nil
+}
+
+// fsnotifyWatcher collects events from a [fsnotify.Watcher] and converts them
+// into batched LSP [protocol.FileEvent]s.
+type fsnotifyWatcher struct {
+ logger *slog.Logger
+ onEvents func([]protocol.FileEvent)
+ onError func(error)
+ interval time.Duration
+
+ stop chan Unit // closed by Close to terminate run and process loop
+ wg sync.WaitGroup // counts the number of active run and process goroutines (max 2)
+
+ ready chan Unit // signals work to process
+
+ watcher *fsnotify.Watcher
+
+ mu sync.Mutex // guards all fields below
+
+ // in is the queue of fsnotify events waiting to be processed.
+ in []fsnotify.Event
+
+ // out is the current batch of unsent file events, which will be sent when
+ // the timer expires.
+ out []protocol.FileEvent
+
+ // knownDirs tracks all known directories to help distinguish between file
+ // and directory deletion events.
+ knownDirs map[string]Unit
+}
+
+// run is the receiver and sender loop.
+//
+// As receiver, its primary responsibility is to drain events and errors from
+// the fsnotify watcher as quickly as possible and enqueue events for processing
+// by the process goroutine. This is critical to work around a potential
+// fsnotify deadlock (see fsnotify/fsnotify#502).
+//
+// As sender, it manages a timer and flush events to the handler if there is
+// no events captured for a period of time.
+func (w *fsnotifyWatcher) run() {
+ timer := time.NewTimer(w.interval)
+ defer timer.Stop()
+
+ for {
+ select {
+ case <-w.stop:
+ return
+
+ case <-timer.C:
+ // TODO(hxjiang): flush is triggered when there is no events captured
+ // in a certain period of time, it may be better to flush it when the
+ // w.in is completely empty.
+ //
+ // Currently, partial events may be emitted if a directory watch gets
+ // stuck. While this does not affect correctness, it means events
+ // might be sent to the client in multiple portions rather than a
+ // single batch.
+ w.mu.Lock()
+ events := w.out
+ w.out = nil
+ w.mu.Unlock()
+
+ if len(events) > 0 {
+ w.onEvents(events)
+ }
+
+ timer.Reset(w.interval)
+
+ case event, ok := <-w.watcher.Events:
+ // The watcher closed. Continue the loop and let the <-w.stop case
+ // handle the actual shutdown.
+ if !ok {
+ continue
+ }
+
+ // TODO(hxjiang): perform some filtering before we reset the timer
+ // to avoid consistenly resetting the timer in a noisy file syestem,
+ // or simply convert the event here.
+ timer.Reset(w.interval)
+
+ w.mu.Lock()
+ w.in = append(w.in, event)
+ w.mu.Unlock()
+
+ w.signal()
+
+ case err, ok := <-w.watcher.Errors:
+ // The watcher closed. Continue the loop and let the <-w.stop case
+ // handle the actual shutdown.
+ if !ok {
+ continue
+ }
+
+ w.onError(err)
+ }
+ }
+}
+
+// process is a worker goroutine that converts raw fsnotify events from queue
+// and handles the potentially blocking work of watching new directories. It is
+// the counterpart to the run goroutine.
+func (w *fsnotifyWatcher) process() {
+ for {
+ select {
+ case <-w.stop:
+ return
+
+ case <-w.ready:
+ w.mu.Lock()
+ events := w.in
+ w.in = nil
+ w.mu.Unlock()
+
+ for _, event := range events {
+ // File watcher is closing, drop any remaining work.
+ select {
+ case <-w.stop:
+ return
+ default:
+ }
+
+ // fsnotify does not guarantee clean filepaths.
+ event.Name = filepath.Clean(event.Name)
+
+ // fsnotify.Event should not be handled concurrently, to preserve their
+ // original order. For example, if a file is deleted and recreated,
+ // concurrent handling could process the events in reverse order.
+ e, isDir := w.convertEvent(event)
+ if e == (protocol.FileEvent{}) {
+ continue
+ }
+
+ var synthesized []protocol.FileEvent // synthesized create events
+
+ if isDir {
+ switch e.Type {
+ case protocol.Created:
+ // Walks the entire directory tree, synthesizes create
+ // events for its contents, and establishes watches for
+ // subdirectories. This recursive, pre-order traversal
+ // guarantees a logical event sequence: parent directory
+ // creation events always precede those of their children.
+ //
+ // For example, consider a creation event for directory
+ // a, and suppose a has contents [a/b, a/b/c, a/c, a/c/d].
+ // The effective events will be:
+ //
+ // CREATE a
+ // CREATE a/b
+ // CREATE a/b/c
+ // CREATE a/c
+ // CREATE a/c/d
+ w.walkDirWithRetry(event.Name, w.onError, func(path string, isDir bool) error {
+ if path != event.Name {
+ synthesized = append(synthesized, protocol.FileEvent{
+ URI: protocol.URIFromPath(path),
+ Type: protocol.Created,
+ })
+ }
+
+ if isDir {
+ return w.watchDir(path)
+ } else {
+ return nil
+ }
+ })
+
+ case protocol.Deleted:
+ // Upon removal, we only need to remove the entries from
+ // the map. The [fsnotify.Watcher] removes the watch for
+ // us. fsnotify/fsnotify#268
+ w.mu.Lock()
+ delete(w.knownDirs, event.Name)
+ w.mu.Unlock()
+ default:
+ // convertEvent enforces that dirs are only Created or Deleted.
+ panic("impossible")
+ }
+ }
+
+ // Discovered events must be appended to the 'out' slice atomically.
+ // This ensures that at any point, the slice contains a logically
+ // correct (maybe slightly outdated) batch of file events that is
+ // ready to be flushed.
+ w.mu.Lock()
+ // Some systems emit duplicate change events in close
+ // succession upon file modification. While the current
+ // deduplication is naive and only handles immediate duplicates,
+ // a more robust solution is needed.
+ // https://github.com/fsnotify/fsnotify?tab=readme-ov-file#why-do-i-get-many-chmod-events
+ //
+ // TODO(hxjiang): Enhance deduplication. The current batching of
+ // events means all duplicates, regardless of proximity, should
+ // be removed. Consider checking the entire buffered slice or
+ // using a map for this.
+ if len(w.out) == 0 || w.out[len(w.out)-1] != e {
+ w.out = append(w.out, e)
+ }
+ w.out = append(w.out, synthesized...) // synthesized events are guaranteed to be unique
+ w.mu.Unlock()
+ }
+ }
+ }
+}
+
+// signal notifies the process goroutine that events are added to the queue and
+// ready for handling.
+func (w *fsnotifyWatcher) signal() {
+ select {
+ case w.ready <- Unit{}:
+ default:
+ }
+}
+
+// skipDir reports whether the input dir should be skipped.
+// Directories that are unlikely to contain Go source files relevant for
+// analysis, such as .git directories or testdata, should be skipped to
+// avoid unnecessary file system notifications. This reduces noise and
+// improves efficiency. Conversely, any directory that might contain Go
+// source code should be watched to ensure that gopls can respond to
+// file changes.
+func skipDir(dirName string) bool {
+ // TODO(hxjiang): the file watcher should honor gopls directory
+ // filter or the new go.mod ignore directive, or actively listening
+ // to gopls register capability request with method
+ // "workspace/didChangeWatchedFiles" like a real LSP client.
+ return strings.HasPrefix(dirName, ".") || strings.HasPrefix(dirName, "_") || dirName == "testdata"
+}
+
+// skipFile reports whether the file should be skipped.
+func skipFile(fileName string) bool {
+ switch strings.TrimPrefix(filepath.Ext(fileName), ".") {
+ case "go", "mod", "sum", "work", "s":
+ return false
+ default:
+ return true
+ }
+}
+
+// WatchDir walks through the directory and all its subdirectories, adding
+// them to the watcher.
+func (w *fsnotifyWatcher) WatchDir(path string) error {
+ return filepath.WalkDir(filepath.Clean(path), func(path string, d fs.DirEntry, err error) error {
+ if d.IsDir() {
+ if skipDir(d.Name()) {
+ return filepath.SkipDir
+ }
+
+ return w.watchDir(path)
+ }
+ return nil
+ })
+}
+
+// convertEvent translates an [fsnotify.Event] into a [protocol.FileEvent].
+// It returns the translated event and a boolean indicating if the path was a
+// directory. For directories, the event Type is either Created or Deleted.
+// It returns empty event for events that should be ignored.
+func (w *fsnotifyWatcher) convertEvent(event fsnotify.Event) (_ protocol.FileEvent, isDir bool) {
+ // Determine if the event is for a directory.
+ if info, err := os.Stat(event.Name); err == nil {
+ isDir = info.IsDir()
+ } else if os.IsNotExist(err) {
+ // Upon deletion, the file/dir has been removed. fsnotify does not
+ // provide information regarding the deleted item.
+ // Use the set of known directories to determine if the deleted item was a directory.
+ isDir = w.isWatchedDir(event.Name)
+ } else {
+ // If statting failed, something is wrong with the file system.
+ // Log and move on.
+ if w.logger != nil {
+ w.logger.Error("failed to stat path, skipping event as its type (file/dir) is unknown", "path", event.Name, "err", err)
+ }
+ return protocol.FileEvent{}, false
+ }
+
+ // Filter out events for directories and files that are not of interest.
+ if isDir && skipDir(filepath.Base(event.Name)) {
+ return protocol.FileEvent{}, true
+ }
+ if !isDir && skipFile(filepath.Base(event.Name)) {
+ return protocol.FileEvent{}, false
+ }
+
+ var t protocol.FileChangeType
+ switch {
+ case event.Op.Has(fsnotify.Rename):
+ // A rename is treated as a deletion of the old path because the
+ // fsnotify RENAME event doesn't include the new path. A separate
+ // CREATE event will be sent for the new path if the destination
+ // directory is watched.
+ fallthrough
+ case event.Op.Has(fsnotify.Remove):
+ // TODO(hxjiang): Directory removal events from some LSP clients may
+ // not include corresponding removal events for child files and
+ // subdirectories. Should we do some filtering when adding the dir
+ // deletion event to the events slice.
+ t = protocol.Deleted
+ case event.Op.Has(fsnotify.Create):
+ t = protocol.Created
+ case event.Op.Has(fsnotify.Write):
+ if isDir {
+ return protocol.FileEvent{}, isDir // ignore dir write events
+ }
+ t = protocol.Changed
+ default:
+ return protocol.FileEvent{}, isDir // ignore the rest of the events
+ }
+
+ return protocol.FileEvent{
+ URI: protocol.URIFromPath(event.Name),
+ Type: t,
+ }, isDir
+}
+
+// watchDir registers a watch for a directory, retrying with backoff if it fails.
+//
+// Returns nil on success or watcher closing; otherwise, the last error after
+// all retries.
+func (w *fsnotifyWatcher) watchDir(path string) error {
+ w.mu.Lock()
+ w.knownDirs[path] = Unit{}
+ w.mu.Unlock()
+
+ // On darwin, watching a directory will fail if it contains broken symbolic
+ // links. This state can occur temporarily during operations like a git
+ // branch switch. To handle this, we retry multiple times with exponential
+ // backoff, allowing time for the symbolic link's target to be created.
+ var (
+ delay = 500 * time.Millisecond
+ err error
+ )
+
+ for i := range 5 {
+ if i > 0 {
+ select {
+ case <-time.After(delay):
+ delay *= 2
+ case <-w.stop:
+ return nil
+ }
+ }
+ // This function may block due to fsnotify/fsnotify#502.
+ err = w.watcher.Add(path)
+ if afterAddHook != nil {
+ afterAddHook(path, err)
+ }
+ if err == nil {
+ break
+ }
+ }
+
+ return err
+}
+
+var afterAddHook func(path string, err error)
+
+// isWatchedDir reports whether the given path is a known directory that
+// the watcher is managing.
+func (w *fsnotifyWatcher) isWatchedDir(path string) bool {
+ w.mu.Lock()
+ defer w.mu.Unlock()
+
+ _, isDir := w.knownDirs[path]
+ return isDir
+}
+
+// Poke is a no-op for fsnotify watcher, as it relies on OS events.
+func (w *fsnotifyWatcher) Poke() {}
+
+// Close shuts down the watcher, waits for the internal goroutine to terminate,
+// and returns any final error.
+func (w *fsnotifyWatcher) Close() error {
+ // Wait for fsnotify' watcher to terminate.
+ err := w.watcher.Close()
+
+ // Wait for run and process loop to terminate. It's important to stop the
+ // run and process loop the last place because we don't know whether
+ // fsnotify's watcher expect us to keep consuming events or errors from
+ // [fsnotify.Watcher.Events] and [fsnotify.Watcher.Errors] while it's being
+ // closed.
+ // To avoid any potential deadlock, have the channel receiver running until
+ // the last minute.
+ close(w.stop)
+ w.wg.Wait()
+
+ return err
+}
+
+// walkDir calls fn against current path and recursively descends path for each
+// file or directory of our interest.
+func (w *fsnotifyWatcher) walkDir(path string, isDir bool, errHandler func(error), fn func(path string, isDir bool) error) {
+ if err := fn(path, isDir); err != nil {
+ errHandler(err)
+ return
+ }
+
+ if !isDir {
+ return
+ }
+
+ entries, err := tryFSOperation(w.stop, func() ([]fs.DirEntry, error) {
+ // ReadDir may fail due because other processes may be actively
+ // modifying the watched dir see golang/go#74820.
+ // TODO(hxjiang): consider adding robustio.ReadDir.
+ return os.ReadDir(path)
+ })
+ if err != nil {
+ if err != ErrClosed {
+ errHandler(err)
+ }
+ return
+ }
+
+ for _, e := range entries {
+ if e.IsDir() && skipDir(e.Name()) {
+ continue
+ }
+ if !e.IsDir() && skipFile(e.Name()) {
+ continue
+ }
+
+ w.walkDir(filepath.Join(path, e.Name()), e.IsDir(), errHandler, fn)
+ }
+}
+
+// walkDirWithRetry walks the file tree rooted at root, calling fn for each
+// file or directory of our interest in the tree, including root.
+//
+// All errors that arise visiting directories or files will be reported to the
+// provided error handler function. If an error is encountered visiting a
+// directory, that entire subtree will be skipped.
+//
+// walkDirWithRetry does not follow symbolic links.
+//
+// It is used instead of [filepath.WalkDir] because it provides control over
+// retry behavior when reading a directory fails. If [os.ReadDir] fails with an
+// ephemeral error, it is retried multiple times with exponential backoff.
+//
+// TODO(hxjiang): call walkDirWithRetry in WalkDir.
+func (w *fsnotifyWatcher) walkDirWithRetry(root string, errHandler func(error), fn func(path string, isDir bool) error) {
+ info, err := tryFSOperation(w.stop, func() (os.FileInfo, error) {
+ return os.Lstat(root) // [os.Lstat] does not follow symlink.
+ })
+ if err != nil {
+ if err != ErrClosed {
+ errHandler(err)
+ }
+ return
+ }
+
+ w.walkDir(root, info.IsDir(), errHandler, fn)
+}
+
+// tryFSOperation executes a function `op` with retry logic, making it resilient
+// to transient errors. It attempts the operation up to 5 times with exponential
+// backoff. Retries occur only if the error is ephemeral.
+//
+// The operation can be interrupted by closing the `stop` channel, in which case
+// it returns [ErrClosed].
+func tryFSOperation[Result any](stop <-chan Unit, op func() (Result, error)) (Result, error) {
+ var (
+ delay = 50 * time.Millisecond
+ err error
+ )
+
+ for i := range 5 {
+ if i > 0 {
+ select {
+ case <-time.After(delay):
+ delay *= 2
+ case <-stop:
+ var zero Result
+ return zero, ErrClosed
+ }
+ }
+
+ var res Result
+ res, err = op()
+
+ if robustio.IsEphemeralError(err) {
+ continue
+ } else {
+ return res, err
+ }
+ }
+
+ var zero Result
+ return zero, err // return last error encountered
+}
diff --git a/gopls/internal/filewatcher/poll_watcher.go b/gopls/internal/filewatcher/poll_watcher.go
new file mode 100644
index 0000000..a25d26f
--- /dev/null
+++ b/gopls/internal/filewatcher/poll_watcher.go
@@ -0,0 +1,353 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+/*
+Design Notes:
+The pollWatcher provides a portable, polling-based alternative to fsnotify.
+Key lessons and design decisions from its implementation:
+
+1. Persistence: It leverages the gopls machine-global filecache to persist
+ file tree state across sessions, keyed by the hash of the root directory.
+2. Adaptive Polling: It uses an adaptive backoff timer that accelerates
+ (to 2s) when user activity is signaled via Poke() and slows down
+ (up to 1m) when the file system is idle.
+3. Synchronous Baseline: WatchDir performs a synchronous initial scan if
+ no cached state exists. This acts as a barrier, ensuring that any
+ subsequent file system changes are correctly detected as events.
+4. Coalescing: Unlike fsnotify, polling naturally coalesces rapid sequences
+ of events (e.g., Create + Change) into a single event based on the
+ state difference between scans.
+5. Multi-root: The watcher supports multiple independent root directories,
+ each with its own independent state and persistence.
+*/
+
+package filewatcher
+
+import (
+ "context"
+ "crypto/sha256"
+ "fmt"
+ "io/fs"
+ "log"
+ "log/slog"
+ "path/filepath"
+ "sync"
+ "time"
+
+ "golang.org/x/sync/singleflight"
+ "golang.org/x/tools/gopls/internal/filecache"
+ "golang.org/x/tools/gopls/internal/protocol"
+ "golang.org/x/tools/gopls/internal/util/bug"
+ "golang.org/x/tools/gopls/internal/util/frob"
+ "golang.org/x/tools/gopls/internal/util/moremaps"
+ "golang.org/x/tools/internal/event"
+)
+
+// NewPollWatcher creates a new watcher that actively polls the file tree to
+// detect changes. It uses an adaptive back-off strategy to reduce scans of the
+// file tree and save battery; it is thus only eventually consistent.
+func NewPollWatcher(interval time.Duration, logger *slog.Logger, onEvents func([]protocol.FileEvent), onError func(error)) *pollWatcher {
+ w := &pollWatcher{
+ logger: logger,
+ onEvents: onEvents,
+ onError: onError,
+ stop: make(chan Unit),
+ poke: make(chan Unit, 1),
+ roots: make(map[string]map[string]fileInfo),
+ interval: interval,
+ }
+ w.wg.Go(w.loop)
+ return w
+}
+
+type pollWatcher struct {
+ logger *slog.Logger
+ onEvents func([]protocol.FileEvent)
+ onError func(error)
+ interval time.Duration // polling interval
+
+ stop chan Unit // closed by Close to terminate run and process loop
+ wg sync.WaitGroup // counts the number of active [pollWatcher.loop] goroutine (max 1)
+
+ poke chan Unit // signals user activity
+
+ loadingGroup singleflight.Group
+
+ mu sync.Mutex // guards field below
+ roots map[string]map[string]fileInfo // root -> relative path -> file info
+}
+
+// fileInfo is a frob-serializable record of the information returned
+// by stat for a single directory entry.
+type fileInfo struct {
+ Mtime int64 // as defined by Time.UnixNano
+ Size int64
+ IsDir bool
+}
+
+func (w *pollWatcher) WatchDir(path string) error {
+ path = filepath.Clean(path)
+
+ w.mu.Lock()
+ _, ok := w.roots[path]
+ w.mu.Unlock()
+ if ok {
+ return nil
+ }
+
+ // Use singleflight to prevent a cache stampede.
+ // If multiple goroutines request to watch the same path concurrently,
+ // only the first will perform the heavy disk scan. The others will block
+ // here and share the result once it completes.
+ _, err, _ := w.loadingGroup.Do(path, func() (any, error) {
+ // Prevent concurrency issue where there are several go routines waiting
+ // to be executed before the first watch dir finishes.
+ w.mu.Lock()
+ _, ok := w.roots[path]
+ w.mu.Unlock()
+ if ok {
+ return nil, nil
+ }
+ return nil, w.watchDir(path)
+ })
+
+ return err
+}
+
+func (w *pollWatcher) watchDir(path string) error {
+ // Load existing state for this root to minimize events on startup.
+ state := w.loadState(path)
+
+ // If no state exists, perform a synchronous scan to establish a baseline.
+ // This ensures that any changes occurring after WatchDir returns are
+ // detected as new events, rather than being treated as "already present".
+ if state == nil {
+ _, newState, err := w.scan(path, nil)
+ if err != nil {
+ return err
+ }
+ state = newState
+ w.saveState(path, state)
+ }
+
+ w.mu.Lock()
+ w.roots[path] = state
+ w.mu.Unlock()
+
+ // Trigger a scan soon to detect subsequent changes.
+ w.Poke()
+
+ return nil
+}
+
+func (w *pollWatcher) Close() error {
+ close(w.stop)
+ w.wg.Wait()
+ return nil
+}
+
+func (w *pollWatcher) Poke() {
+ select {
+ case w.poke <- Unit{}:
+ default:
+ }
+}
+
+// loop scans the tree periodically, using adaptive backoff, until the watcher is closed.
+// A call to Poke
+func (w *pollWatcher) loop() {
+ delay := w.interval
+ timer := time.NewTimer(delay)
+ defer timer.Stop()
+
+ for {
+ select {
+ case <-w.stop:
+ return
+
+ case <-w.poke:
+ delay = w.interval
+ if !timer.Stop() {
+ // Failed to stop, drain the channel.
+ select {
+ case <-timer.C:
+ default:
+ }
+ }
+ timer.Reset(delay)
+
+ case <-timer.C:
+ // Scan all roots
+ w.mu.Lock()
+ roots := moremaps.KeySlice(w.roots)
+ w.mu.Unlock()
+
+ changed := false
+ for _, root := range roots {
+ w.mu.Lock()
+ state, ok := w.roots[root]
+ w.mu.Unlock()
+ if !ok {
+ continue // Root removed (unlikely currently as we don't support RemoveWatch)
+ }
+
+ changes, newState, err := w.scan(root, state)
+ if err != nil {
+ if w.onError != nil {
+ w.onError(err)
+ }
+ // Continue to next root
+ continue
+ }
+
+ w.mu.Lock()
+ if _, ok := w.roots[root]; ok {
+ w.roots[root] = newState
+ }
+ w.mu.Unlock()
+
+ if len(changes) > 0 {
+ w.onEvents(changes)
+ w.saveState(root, newState)
+ changed = true
+ } else if state == nil {
+ // Initial baseline established, save it so next run has a comparison.
+ w.saveState(root, newState)
+ }
+ }
+
+ if changed {
+ // If changes found, keep polling fast for a bit.
+ delay = w.interval
+ } else {
+ // No changes, backoff.
+ delay = min(delay*2, 1*time.Minute)
+ }
+ timer.Reset(delay)
+ }
+ }
+}
+
+// scan walks the file tree for the given root directory and compares its
+// current state against the provided oldState, returning a coalesced list
+// of file system events (Created, Changed, Deleted) and the new state map.
+//
+// This method is concurrency safe and may be triggered concurrently (e.g., by
+// multiple "scan" calls). It does not mutate the watcher's internal state.
+//
+// To prevent triggering massive workspace reloads in the LSP, scan explicitly
+// ignores modification time changes on the root directory itself.
+func (w *pollWatcher) scan(root string, oldState map[string]fileInfo) ([]protocol.FileEvent, map[string]fileInfo, error) {
+ // Debug purpose.
+ {
+ t0 := time.Now()
+ log.Printf("scan %s", root)
+ defer func() {
+ log.Printf("scan %s done: %v", root, time.Since(t0))
+ }()
+ }
+
+ var (
+ newState = make(map[string]fileInfo)
+ events []protocol.FileEvent
+ )
+ err := filepath.WalkDir(root, func(path string, e fs.DirEntry, err error) error {
+ if err != nil {
+ // Permission errors or disappearing files are ignored during walk.
+ return nil
+ }
+ if path == root {
+ // Skip the root directory itself. We are interested in its contents.
+ // This avoids emitting a "Changed" event for the root whenever a
+ // file is added or removed.
+ return nil
+ }
+ if e.IsDir() && skipDir(e.Name()) {
+ return filepath.SkipDir
+ }
+ if !e.IsDir() && skipFile(e.Name()) {
+ return nil
+ }
+
+ info, err := e.Info()
+ if err != nil {
+ return nil
+ }
+
+ newInfo := fileInfo{
+ Mtime: info.ModTime().UnixNano(),
+ Size: info.Size(),
+ IsDir: e.IsDir(),
+ }
+ newState[path] = newInfo
+
+ if oldState == nil { // Initial population, no events.
+ return nil
+ }
+
+ if old, ok := oldState[path]; ok { // Change event.
+ if old.Mtime != newInfo.Mtime || old.Size != newInfo.Size || old.IsDir != newInfo.IsDir {
+ events = append(events, protocol.FileEvent{
+ URI: protocol.URIFromPath(path),
+ Type: protocol.Changed,
+ })
+ }
+ } else { // Creation event.
+ events = append(events, protocol.FileEvent{
+ URI: protocol.URIFromPath(path),
+ Type: protocol.Created,
+ })
+ }
+ return nil
+ })
+ if err != nil {
+ return nil, nil, err
+ }
+
+ // Deletion events.
+ for path := range oldState {
+ if _, ok := newState[path]; !ok {
+ events = append(events, protocol.FileEvent{
+ URI: protocol.URIFromPath(path),
+ Type: protocol.Deleted,
+ })
+ }
+ }
+
+ return events, newState, nil
+}
+
+// -- filecache --
+
+func (w *pollWatcher) loadState(root string) map[string]fileInfo {
+ key := cacheKey(root)
+ data, err := filecache.Get(filewatcherKind, key)
+ if err != nil {
+ if err != filecache.ErrNotFound {
+ bug.Reportf("internal error reading shared cache: %v", err)
+ }
+ return nil
+ }
+
+ var state map[string]fileInfo
+ codec.Decode(data, &state)
+ return state
+}
+
+func (w *pollWatcher) saveState(root string, state map[string]fileInfo) {
+ key := cacheKey(root)
+ data := codec.Encode(state)
+ if err := filecache.Set(filewatcherKind, key, data); err != nil {
+ ctx := context.Background() // FIXME
+ event.Error(ctx, fmt.Sprintf("pollWatcher.saveState(%s)", root), err)
+ }
+}
+
+const filewatcherKind = "filewatcher"
+
+var codec = frob.CodecFor[map[string]fileInfo]()
+
+func cacheKey(root string) [32]byte {
+ return sha256.Sum256([]byte(root))
+}
diff --git a/gopls/internal/server/general.go b/gopls/internal/server/general.go
index 56e4eb7..4eba399 100644
--- a/gopls/internal/server/general.go
+++ b/gopls/internal/server/general.go
@@ -19,12 +19,15 @@
"sort"
"strings"
"sync"
+ "time"

"golang.org/x/telemetry/counter"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/debug"
debuglog "golang.org/x/tools/gopls/internal/debug/log"
+ "golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/filecache"
+ "golang.org/x/tools/gopls/internal/filewatcher"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/semtok"
"golang.org/x/tools/gopls/internal/settings"
@@ -35,6 +38,7 @@
"golang.org/x/tools/gopls/internal/util/moreslices"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/jsonrpc2"
+ "golang.org/x/tools/internal/xcontext"
)

func (s *server) Initialize(ctx context.Context, params *protocol.ParamInitialize) (*protocol.InitializeResult, error) {
@@ -415,6 +419,11 @@
func (s *server) updateWatchedDirectories(ctx context.Context) error {
patterns := s.session.FileWatchingGlobPatterns(ctx)

+ if err := s.updateServerSideWatcher(ctx, patterns); err != nil {
+ // FIXME(adonovan): return the error?
+ event.Error(ctx, "failed to update server-side file watcher", err)
+ }
+
s.watchedGlobPatternsMu.Lock()
defer s.watchedGlobPatternsMu.Unlock()

@@ -492,6 +501,68 @@
return nil
}

+func (s *server) updateServerSideWatcher(ctx context.Context, patterns map[protocol.RelativePattern]unit) error {
+ mode := s.Options().FileWatcher
+ s.fileWatcherMu.Lock()
+ defer s.fileWatcherMu.Unlock()
+
+ if mode == "" {
+ // FIXME: do this when settings change.
+ if s.fileWatcher != nil {
+ s.fileWatcher.Close() // FIXME don't ignore error
+ s.fileWatcher = nil
+ }
+ return nil
+ }
+
+ if s.fileWatcher == nil {
+ onChange := func(events []protocol.FileEvent) {
+ // We don't have a context here, but didModifyFiles needs one.
+ // We can use a background context.
+ // TODO: We need to ensure we don't process events after shutdown.
+ ctx := xcontext.Detach(ctx)
+ var modifications []file.Modification
+ for _, e := range events {
+ modifications = append(modifications, file.Modification{
+ URI: e.URI,
+ Action: changeTypeToFileAction(e.Type),
+ OnDisk: true,
+ })
+ }
+ if len(modifications) > 0 {
+ if err := s.didModifyFiles(ctx, modifications, FromDidChangeWatchedFiles); err != nil {
+ event.Error(ctx, "failed to process file changes", err)
+ }
+ }
+ }
+ // Start the watcher.
+ // Use a long delay (100ms) to coalesce events.
+ w, err := filewatcher.New(mode, 100*time.Millisecond, nil, onChange, func(err error) {
+ event.Error(ctx, "file watcher error", err)
+ })
+ if err != nil {
+ return err
+ }
+ s.fileWatcher = w
+ }
+
+ // Update the set of watched directories.
+ // We extract unique directories from the patterns.
+ dirs := make(map[string]struct{})
+ for pattern := range patterns {
+ if pattern.BaseURI != "" {
+ dirs[pattern.BaseURI.Path()] = struct{}{}
+ }
+ }
+ for dir := range dirs {
+ if err := s.fileWatcher.WatchDir(dir); err != nil {
+ // Log warning but continue watching other directories.
+ event.Log(ctx, fmt.Sprintf("failed to watch directory %s: %v", dir, err))
+ }
+ }
+ return nil
+}
+
// Options returns the current server options.
//
// The caller must not modify the result.
diff --git a/gopls/internal/server/server.go b/gopls/internal/server/server.go
index 71eb331..6ce3df03 100644
--- a/gopls/internal/server/server.go
+++ b/gopls/internal/server/server.go
@@ -27,6 +27,7 @@

"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
+ "golang.org/x/tools/gopls/internal/filewatcher"
"golang.org/x/tools/gopls/internal/golang"
"golang.org/x/tools/gopls/internal/golang/splitpkg"
"golang.org/x/tools/gopls/internal/progress"
@@ -111,6 +112,9 @@
watchedGlobPatterns map[protocol.RelativePattern]unit
watchRegistrationCount int

+ fileWatcherMu sync.Mutex
+ fileWatcher filewatcher.Watcher
+
diagnosticsMu sync.Mutex // guards map and its values
diagnostics map[protocol.DocumentURI]*fileDiagnostics

diff --git a/gopls/internal/server/text_synchronization.go b/gopls/internal/server/text_synchronization.go
index 1e8ebfd..9c92515 100644
--- a/gopls/internal/server/text_synchronization.go
+++ b/gopls/internal/server/text_synchronization.go
@@ -219,6 +219,13 @@
}

func (s *server) didModifyFiles(ctx context.Context, modifications []file.Modification, cause ModificationSource) error {
+ // Something happened. Wake up a quiescent file watcher.
+ s.fileWatcherMu.Lock()
+ if s.fileWatcher != nil {
+ s.fileWatcher.Poke()
+ }
+ s.fileWatcherMu.Unlock()
+
// wg guards two conditions:
// 1. didModifyFiles is complete
// 2. the goroutine diagnosing changes on behalf of didModifyFiles is
diff --git a/gopls/internal/settings/settings.go b/gopls/internal/settings/settings.go
index 652aed8..e511ea1 100644
--- a/gopls/internal/settings/settings.go
+++ b/gopls/internal/settings/settings.go
@@ -792,6 +792,12 @@
// issue.
SubdirWatchPatterns SubdirWatchPatterns

+ // FileWatcher controls the file watching implementation.
+ //
+ // The default value is "", which disables server-side file watching.
+ // Other values are "fsnotify" and "poll".
+ FileWatcher string
+
// ReportAnalysisProgressAfter sets the duration for gopls to wait before starting
// progress reporting for ongoing go/analysis passes.
//

Change information

Files:
  • M gopls/internal/cmd/mcp.go
  • M gopls/internal/filewatcher/filewatcher.go
  • M gopls/internal/filewatcher/filewatcher_test.go
  • A gopls/internal/filewatcher/fsnotify_watcher.go
  • A gopls/internal/filewatcher/poll_watcher.go
  • M gopls/internal/server/general.go
  • M gopls/internal/server/server.go
  • M gopls/internal/server/text_synchronization.go
  • M gopls/internal/settings/settings.go
Change size: XL
Delta: 9 files changed, 1273 insertions(+), 710 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: Ieaac1cbc6ccc0ac1d0fc71dc9c48037a4dcf8a24
Gerrit-Change-Number: 754320
Gerrit-PatchSet: 1
Gerrit-Owner: Hongxiang Jiang <hxj...@golang.org>
Gerrit-CC: Alan Donovan <adon...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Hongxiang Jiang (Gerrit)

unread,
Mar 18, 2026, 6:41:34 AM (yesterday) Mar 18
to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Alan Donovan

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:
  • Alan Donovan
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: Ieaac1cbc6ccc0ac1d0fc71dc9c48037a4dcf8a24
    Gerrit-Change-Number: 754320
    Gerrit-PatchSet: 15
    Gerrit-Owner: Hongxiang Jiang <hxj...@golang.org>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Hongxiang Jiang <hxj...@golang.org>
    Gerrit-Attention: Alan Donovan <adon...@google.com>
    Gerrit-Comment-Date: Wed, 18 Mar 2026 10:41:24 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Hongxiang Jiang (Gerrit)

    unread,
    Mar 18, 2026, 6:41:54 AM (yesterday) Mar 18
    to Alan Donovan, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Hongxiang Jiang uploaded new patchset

    Hongxiang Jiang uploaded patch set #16 to this change.
    Following approvals got outdated and were removed:
    • TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: newpatchset
      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: Ieaac1cbc6ccc0ac1d0fc71dc9c48037a4dcf8a24
      Gerrit-Change-Number: 754320
      Gerrit-PatchSet: 16
      Gerrit-Owner: Hongxiang Jiang <hxj...@golang.org>
      Gerrit-Reviewer: Hongxiang Jiang <hxj...@golang.org>
      Gerrit-CC: Alan Donovan <adon...@google.com>
      unsatisfied_requirement
      satisfied_requirement
      open
      diffy

      Hongxiang Jiang (Gerrit)

      unread,
      Mar 18, 2026, 6:44:50 AM (yesterday) Mar 18
      to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com

      Hongxiang Jiang added 1 comment

      Commit Message
      Line 22, Patchset 16 (Latest):Windows uses mandatory file locking, when the poll watcher is scanning
      the dir, other processes can not operate on the same file. The file
      watcher's test have also changed to try with backoff to make sure the
      operation can survive any transient error.
      Hongxiang Jiang . unresolved

      @adon...@google.com

      I think this is the only concern with the current solution. There isn't any other draw backs from the poll based solution.

      I observed one failure in the previous commit:

      https://logs.chromium.org/logs/golang/buildbucket/cr-buildbucket/8687073485957395121/+/u/step/36/log/2

      I think the other processes (the user or on behalf of the user) may hit similar issues if the other processes does not have such "retry" or "ephemeral" tolerance.

      Open in Gerrit

      Related details

      Attention set is empty
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: Ieaac1cbc6ccc0ac1d0fc71dc9c48037a4dcf8a24
        Gerrit-Change-Number: 754320
        Gerrit-PatchSet: 16
        Gerrit-Owner: Hongxiang Jiang <hxj...@golang.org>
        Gerrit-Reviewer: Hongxiang Jiang <hxj...@golang.org>
        Gerrit-CC: Alan Donovan <adon...@google.com>
        Gerrit-Comment-Date: Wed, 18 Mar 2026 10:44:44 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        unsatisfied_requirement
        open
        diffy

        Alan Donovan (Gerrit)

        unread,
        Mar 18, 2026, 9:51:49 PM (14 hours ago) Mar 18
        to Hongxiang Jiang, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
        Attention needed from Alan Donovan and Hongxiang Jiang

        Alan Donovan added 32 comments

        Patchset-level comments
        File-level comment, Patchset 16 (Latest):
        Alan Donovan . resolved

        Thanks for tackling this. I still suspect there is a way to express it without so many critical sections to access w.roots, but this looks good.

        File gopls/internal/filewatcher/filewatcher_test.go
        Line 37, Patchset 16 (Latest): time.Sleep(delay)
        delay *= 2
        Alan Donovan . unresolved

        This logic belongs just before the continue. Then you don't need i.

        Line 42, Patchset 16 (Latest): if robustio.IsEphemeralError(err) {
        continue
        } else {
        return err
        }
        Alan Donovan . unresolved

        Nit: avoid ending a loop body with an unconditional control statement (such as both arms of this if/else). Prefer implicit `continue`:
        ```
        if !IsEphemeral(err) { return err }
        ```

        Line 182, Patchset 16 (Latest): for _, f := range archive.Files {
        path := filepath.Join(root, f.Name)

        if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
        t.Fatal(err)

        }
        if err := os.WriteFile(path, f.Data, 0644); err != nil {
        t.Fatal(err)
        }
        }
        Alan Donovan . unresolved

        `root := testfiles.CopyToTmp(t, f)`?

        Line 247, Patchset 16 (Latest): // test runs too fast, mtime might not change (if resolution
        Alan Donovan . resolved

        Beware that on FAT file systems the granularity is 2s. I doubt they are much used for source trees today though.

        Line 255, Patchset 16 (Latest): // If the test attempts to modify files at the exact moment
        Alan Donovan . unresolved

        while

        (There are no exact moments of simultaneity, either in computers or in physics.)

        Line 261, Patchset 16 (Latest): // However this also means other process (maybe the user)
        // will encounter this problem while the poll watcher is
        // scanning through the dir.
        Alan Donovan . resolved

        Windows: what a mess.

        I wonder how serious a problem this will be in practice.

        Line 276, Patchset 16 (Latest): want.WriteString(fmt.Sprintf("URI: %s type: %v\n", e.URI, e.Type))
        Alan Donovan . unresolved

        fmt.Fprintf(&want

        Line 282, Patchset 16 (Latest): t.Errorf("found %v matching events slice\nwant sequences:\n%s\nall got:\n%s", len(tt.expectedEvents)-len(all), want.String(), got.String())
        Alan Donovan . unresolved

        &want, &got

        (%s calls .String())

        File gopls/internal/filewatcher/poll_watcher.go
        Line 6, Patchset 16 (Latest):Design Notes:
        Alan Donovan . unresolved

        Let's move this down to L45 (but still a floating comment).

        Line 49, Patchset 16 (Latest):func NewPollWatcher(interval time.Duration, logger *slog.Logger, onEvents func([]protocol.FileEvent), onError func(error)) *pollWatcher {
        Alan Donovan . unresolved

        // The caller must eventually Close the watcher.

        Line 49, Patchset 16 (Latest):func NewPollWatcher(interval time.Duration, logger *slog.Logger, onEvents func([]protocol.FileEvent), onError func(error)) *pollWatcher {
        Alan Donovan . unresolved

        log

        Line 51, Patchset 16 (Latest): logger: logger,
        Alan Donovan . unresolved

        log

        Line 57, Patchset 16 (Latest): roots: make(map[string]map[string]fileInfo),
        Alan Donovan . unresolved

        This type wants a name (fileState) since it appears a lot.

        Line 70, Patchset 16 (Latest): // TODO(hxjiang): accept ctx from constructor and use ctx.Done() for Close.
        ctx context.Context
        Alan Donovan . unresolved

        Shall we do this in this CL?

        Line 73, Patchset 16 (Latest): stop chan struct{} // closed by Close to terminate run and process loop
        Alan Donovan . unresolved

        run

        Line 74, Patchset 16 (Latest): wg sync.WaitGroup // counts the number of active [pollWatcher.loop] goroutine (max 1)
        Alan Donovan . unresolved

        `loops` (since it's a counting semaphore)?

        Line 83, Patchset 16 (Latest): roots map[string]map[string]fileInfo // root -> relative path -> file info
        Alan Donovan . unresolved

        clean root dir ->

        Line 94, Patchset 16 (Latest):func (w *pollWatcher) WatchDir(path string) error {
        Alan Donovan . unresolved

        dir

        Line 95, Patchset 16 (Latest): // TODO(hxjiang): prevent watching for a dir if the parent dir is already
        // being watched.
        Alan Donovan . unresolved

        Isn't this done below?

        Line 100, Patchset 16 (Latest): w.mu.Lock()
        Alan Donovan . unresolved
        You can implement the singleflight semantics easily using the existing map if you make the map items be "futures" for the watchDir call, i.e. `struct { value T; error err; ready chan struct{} }`.
        ```

        func (w *pollWatcher) WatchDir(path string) error {
        	path = filepath.Clean(path)
        	// Use a future cache to suppress duplicate
        // calls to watchDir(path).
        w.mu.Lock()
        fu, ok := w.roots[path]
        if !ok {
        // First time: this goroutine does the work.
        fu = &future{
        ready: make(chan struct{}),
        }
        w.futures[path] = fu
        w.mu.Unlock()

        fu.value, fu.err = w.watchDir(path)
        close(fu.ready) // broadcast ready state
        } else {
        // Some other goroutine got there first.
        w.mu.Unlock()
        <-fi.ready
        }
        // Inv: future is ready.
        return fu.value, fu.err
        }
        ```

        // Failed to stop, drain the channel.
        select {
        case <-timer.C:
        default:
        }
        }
        Alan Donovan . unresolved

        Is this needed? We're about to call Reset.

        Line 212, Patchset 16 (Latest): // Continue to next root
        Alan Donovan . unresolved

        delete

        Line 237, Patchset 16 (Latest): delay = min(delay*2, 1*time.Minute)
        Alan Donovan . unresolved

        A minute may be a long time while editing, but it's not a lot if the machine is idle: we'll just be adding load to the machine for no reason.

        I think this value could be much higher, perhaps hours, so long as any actual activity by the user (a DidChange from the client) causes a Poke.

        Line 248, Patchset 16 (Latest):// This method is concurrency safe and may be triggered concurrently (e.g., by

        // multiple "scan" calls). It does not mutate the watcher's internal state.
        Alan Donovan . unresolved

        I'm confused; this method is scan.

        Line 253, Patchset 16 (Latest):func (w *pollWatcher) scan(root string, oldState map[string]fileInfo) ([]protocol.FileEvent, map[string]fileInfo, error) {
        Alan Donovan . unresolved

        delete

        Then it's self-evident that it doesn't mutate the watcher.

        Line 257, Patchset 16 (Latest): )
        Alan Donovan . unresolved
        Factor:
        ```
        addEvent := func(typ FileChangeType, path string) {
        events = append(events, protocol.FileEvent{
        URI: protocol.URIFromPath(path),
        Type: typ,
        })
        )
        ```
        Line 258, Patchset 16 (Latest): err := filepath.WalkDir(root, func(path string, e fs.DirEntry, err error) error {
        Alan Donovan . unresolved

        dirent

        is the traditional name.

        Line 282, Patchset 16 (Latest): Mtime: info.ModTime().UnixNano(),
        Alan Donovan . unresolved

        ModTime

        Line 292, Patchset 16 (Latest): if old, ok := oldState[path]; ok { // Change event.
        Alan Donovan . unresolved

        These three comments will become self-evident after the addEvent factoring.

        Line 293, Patchset 16 (Latest): if old.Mtime != newInfo.Mtime || old.Size != newInfo.Size || old.IsDir != newInfo.IsDir {
        Alan Donovan . unresolved

        old != new

        Line 326, Patchset 16 (Latest):func (w *pollWatcher) loadState(root string) map[string]fileInfo {
        Alan Donovan . unresolved

        Add an error result, since a nil map is a valid empty map.

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Alan Donovan
        • Hongxiang Jiang
        Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: Ieaac1cbc6ccc0ac1d0fc71dc9c48037a4dcf8a24
        Gerrit-Change-Number: 754320
        Gerrit-PatchSet: 16
        Gerrit-Owner: Hongxiang Jiang <hxj...@golang.org>
        Gerrit-Reviewer: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Hongxiang Jiang <hxj...@golang.org>
        Gerrit-Attention: Hongxiang Jiang <hxj...@golang.org>
        Gerrit-Attention: Alan Donovan <adon...@google.com>
        Gerrit-Comment-Date: Thu, 19 Mar 2026 01:51:45 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        unsatisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages