Test buffered watcher

.circleci/config.yml

@@ -22,8 +22,10 @@ jobs:
           name: test
           command: |
             uname -a
+            sysctl fs.inotify.max_user_watches fs.inotify.max_user_instances
+            ulimit -a
             go version
-            go test -race ./...
+            go test -race -parallel 1 ./...
 
   # iOS
   ios:
@@ -48,7 +50,7 @@ jobs:
             export PATH=$PATH:/usr/local/Cellar/go/*/bin
             uname -a
             go version
-            go test -race ./...
+            go test -race -parallel 1 ./...
 
   # This is just Linux x86_64; also need to get a Go with GOOS=android, but
   # there aren't any pre-built versions of that on the Go site. Idk, disable for
@@ -76,5 +78,5 @@ jobs:
   #           uname -a
   #           export PATH=/usr/local/go/bin:$PATH
   #           go version
-  #           go test -race ./...
+  #           go test -race -parallel 1 ./...
   #

.cirrus.yml

@@ -9,4 +9,4 @@ freebsd_task:
       # run tests as user "cirrus" instead of root
     - pw useradd cirrus -m
     - chown -R cirrus:cirrus .
-    - sudo -u cirrus go test -race ./...
+    - sudo -u cirrus go test -race -parallel 1 ./...

.github/workflows/test.yml

@@ -29,7 +29,7 @@ jobs:
 
       - name: test
         run: |
-          go test -race ./...
+          go test -race -parallel 1 ./...
 
   # Test gccgo
   testgcc:
@@ -42,7 +42,7 @@ jobs:
       - name: test
         run: |
           sudo apt-get -y install gccgo-12
-          go-12 test ./...
+          go-12 test -parallel 1 ./...
 
   # Test only the latest Go version on macOS; we use the macOS builders for BSD
   # and illumos, and GitHub doesn't allow many of them to run concurrently. If
@@ -70,7 +70,7 @@ jobs:
 
       - name: test
         run: |
-          go test -race ./...
+          go test -race -parallel 1 ./...
 
   # OpenBSD; no -race as the VM doesn't include the comp set.
   #
@@ -91,7 +91,7 @@ jobs:
           prepare: pkg_add go
           run: |
             useradd -mG wheel action
-            su action -c 'go test ./...'
+            su action -c 'go test -parallel 1 ./...'
 
   # NetBSD
   testNetBSD:
@@ -107,7 +107,7 @@ jobs:
           # TODO: no -race for the same reason as OpenBSD (the timing; it does run).
           run: |
             useradd -mG wheel action
-            su action -c 'go120 test ./...'
+            su action -c 'go120 test -parallel 1 ./...'
 
   # illumos
   testillumos:
@@ -125,7 +125,7 @@ jobs:
           useradd action
           export GOCACHE=/tmp/go-cache
           export GOPATH=/tmp/go-path
-          su action -c '/opt/ooce/go-1.19/bin/go test ./...'
+          su action -c '/opt/ooce/go-1.19/bin/go test -parallel 1 ./...'
 
   # Older Debian 6, for old Linux kernels.
   testDebian6:
@@ -159,6 +159,10 @@ jobs:
             go test -c -o ${p//\//-}.test $p
           done
           vagrant up
+
+          vagrant ssh -c 'uname -a'
+          vagrant ssh -c 'sysctl fs.inotify.max_user_watches fs.inotify.max_user_instances'
+          vagrant ssh -c 'ulimit -a'
           for t in *.test; do
-            vagrant ssh -c "/vagrant/$t"
+            vagrant ssh -c "/vagrant/$t -test.parallel 1"
           done

backend_fen.go

@@ -136,14 +136,15 @@ func NewWatcher() (*Watcher, error) {
 	return NewBufferedWatcher(0)
 }
 
-// NewBufferedWatcher creates a new Watcher with an optionally buffered event channel.
-// For almost all use cases an unbuffered Watcher will perform better than buffered.
-// Most kernels have de-duplication logic which allows for less activity in userspace
-// and generally better performance.  However there may be some cases where a very
-// large buffers can enable an application to keep up with mass file rotations.
-// You will always be better off increasing the kernel buffers over adding a large
-// userspace buffer, but if you can't control the kernel buffer then a buffered
-// watcher is a reasonable option.  You probably want NewWatcher.
+// NewBufferedWatcher creates a new Watcher with a buffered event channel.
+//
+// For almost all use cases an unbuffered Watcher will perform better; most
+// kernels have de-duplication logic, which means less activity in userspace and
+// generally better performance. However there may be some cases where a very
+// large buffer can enable an application to keep up with a very large number of
+// events. You will always be better off increasing the kernel buffers over
+// adding a large userspace buffer, but if you can't control the kernel buffer
+// then a buffered watcher is a reasonable option.
 func NewBufferedWatcher(sz uint) (*Watcher, error) {
 	w := &Watcher{
 		Events:  make(chan Event, sz),

backend_inotify.go

@@ -235,14 +235,15 @@ func NewWatcher() (*Watcher, error) {
 	return NewBufferedWatcher(0)
 }
 
-// NewBufferedWatcher creates a new Watcher with an optionally buffered event channel.
-// For almost all use cases an unbuffered Watcher will perform better than buffered.
-// Most kernels have de-duplication logic which allows for less activity in userspace
-// and generally better performance.  However there may be some cases where a very
-// large buffers can enable an application to keep up with mass file rotations.
-// You will always be better off increasing the kernel buffers over adding a large
-// userspace buffer, but if you can't control the kernel buffer then a buffered
-// watcher is a reasonable option.  You probably want NewWatcher.
+// NewBufferedWatcher creates a new Watcher with a buffered event channel.
+//
+// For almost all use cases an unbuffered Watcher will perform better; most
+// kernels have de-duplication logic, which means less activity in userspace and
+// generally better performance. However there may be some cases where a very
+// large buffer can enable an application to keep up with a very large number of
+// events. You will always be better off increasing the kernel buffers over
+// adding a large userspace buffer, but if you can't control the kernel buffer
+// then a buffered watcher is a reasonable option.
 func NewBufferedWatcher(sz uint) (*Watcher, error) {
 	// Need to set nonblocking mode for SetDeadline to work, otherwise blocking
 	// I/O operations won't terminate on close.

backend_kqueue.go

@@ -147,14 +147,15 @@ func NewWatcher() (*Watcher, error) {
 	return NewBufferedWatcher(0)
 }
 
-// NewBufferedWatcher creates a new Watcher with an optionally buffered event channel.
-// For almost all use cases an unbuffered Watcher will perform better than buffered.
-// Most kernels have de-duplication logic which allows for less activity in userspace
-// and generally better performance.  However there may be some cases where a very
-// large buffers can enable an application to keep up with mass file rotations.
-// You will always be better off increasing the kernel buffers over adding a large
-// userspace buffer, but if you can't control the kernel buffer then a buffered
-// watcher is a reasonable option.  You probably want NewWatcher.
+// NewBufferedWatcher creates a new Watcher with a buffered event channel.
+//
+// For almost all use cases an unbuffered Watcher will perform better; most
+// kernels have de-duplication logic, which means less activity in userspace and
+// generally better performance. However there may be some cases where a very
+// large buffer can enable an application to keep up with a very large number of
+// events. You will always be better off increasing the kernel buffers over
+// adding a large userspace buffer, but if you can't control the kernel buffer
+// then a buffered watcher is a reasonable option.
 func NewBufferedWatcher(sz uint) (*Watcher, error) {
 	kq, closepipe, err := newKqueue()
 	if err != nil {

backend_other.go

@@ -122,14 +122,15 @@ func NewWatcher() (*Watcher, error) {
 	return nil, errors.New("fsnotify not supported on the current platform")
 }
 
-// NewBufferedWatcher creates a new Watcher with an optionally buffered event channel.
-// For almost all use cases an unbuffered Watcher will perform better than buffered.
-// Most kernels have de-duplication logic which allows for less activity in userspace
-// and generally better performance.  However there may be some cases where a very
-// large buffers can enable an application to keep up with mass file rotations.
-// You will always be better off increasing the kernel buffers over adding a large
-// userspace buffer, but if you can't control the kernel buffer then a buffered
-// watcher is a reasonable option.  You probably want NewWatcher.
+// NewBufferedWatcher creates a new Watcher with a buffered event channel.
+//
+// For almost all use cases an unbuffered Watcher will perform better; most
+// kernels have de-duplication logic, which means less activity in userspace and
+// generally better performance. However there may be some cases where a very
+// large buffer can enable an application to keep up with a very large number of
+// events. You will always be better off increasing the kernel buffers over
+// adding a large userspace buffer, but if you can't control the kernel buffer
+// then a buffered watcher is a reasonable option.
 func NewBufferedWatcher(sz uint) (*Watcher, error) {
 	return NewWatcher() //just re-use the original error response
 }

backend_windows.go

@@ -146,14 +146,15 @@ func NewWatcher() (*Watcher, error) {
 	return NewBufferedWatcher(50) // Windows backend defaults to a buffered channel of size 50
 }
 
-// NewBufferedWatcher creates a new Watcher with an optionally buffered event channel.
-// For almost all use cases an unbuffered Watcher will perform better than buffered.
-// Most kernels have de-duplication logic which allows for less activity in userspace
-// and generally better performance.  However there may be some cases where a very
-// large buffers can enable an application to keep up with mass file rotations.
-// You will always be better off increasing the kernel buffers over adding a large
-// userspace buffer, but if you can't control the kernel buffer then a buffered
-// watcher is a reasonable option.  You probably want NewWatcher.
+// NewBufferedWatcher creates a new Watcher with a buffered event channel.
+//
+// For almost all use cases an unbuffered Watcher will perform better; most
+// kernels have de-duplication logic, which means less activity in userspace and
+// generally better performance. However there may be some cases where a very
+// large buffer can enable an application to keep up with a very large number of
+// events. You will always be better off increasing the kernel buffers over
+// adding a large userspace buffer, but if you can't control the kernel buffer
+// then a buffered watcher is a reasonable option.
 func NewBufferedWatcher(sz uint) (*Watcher, error) {
 	port, err := windows.CreateIoCompletionPort(windows.InvalidHandle, 0, 0, 0)
 	if err != nil {

fsnotify_test.go

@@ -28,6 +28,25 @@ func init() {
 	internal.SetRlimit()
 }
 
+// Quick but somewhat ugly way to run tests against both the standard fsnotify
+// and the buffered one. Should rewrite the tests to run more than once, but
+// effort...
+func TestMain(m *testing.M) {
+	c1 := m.Run()
+	os.Setenv("FSNOTIFY_BUFFER", "1")
+	c2 := m.Run()
+	os.Setenv("FSNOTIFY_BUFFER", "4096")
+	c3 := m.Run()
+
+	if c1 != 0 {
+		os.Exit(c1)
+	}
+	if c2 != 0 {
+		os.Exit(c2)
+	}
+	os.Exit(c3)
+}
+
 func TestWatch(t *testing.T) {
 	tests := []testCase{
 		{"multiple creates", func(t *testing.T, w *Watcher, tmp string) {
@@ -948,20 +967,27 @@ func TestClose(t *testing.T) {
 			time.Sleep(50 * time.Millisecond)
 		}
 
-		select {
-		default:
-			t.Fatal("blocking on Events")
-		case _, ok := <-w.Events:
-			if ok {
-				t.Fatal("Events not closed")
+		tim := time.NewTimer(50 * time.Millisecond)
+	loop:
+		for {
+			select {
+			default:
+				t.Fatal("blocking on Events")
+			case <-tim.C:
+				t.Fatalf("Events not closed")
+			case _, ok := <-w.Events:
+				if !ok {
+					break loop
+				}
 			}
 		}
+
 		select {
 		default:
 			t.Fatal("blocking on Errors")
-		case _, ok := <-w.Errors:
+		case err, ok := <-w.Errors:
 			if ok {
-				t.Fatal("Errors not closed")
+				t.Fatalf("Errors not closed; read:\n\t%s", err)
 			}
 		}
 	}
@@ -1001,6 +1027,7 @@ func TestClose(t *testing.T) {
 
 		touch(t, tmp, "file")
 		rm(t, tmp, "file")
+		eventSeparator()
 		if err := w.Close(); err != nil {
 			t.Fatal(err)
 		}

helpers_test.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"sort"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -46,6 +47,14 @@ func waitForEvents()  { time.Sleep(500 * time.Millisecond) }
 func newWatcher(t *testing.T, add ...string) *Watcher {
 	t.Helper()
 	w, err := NewWatcher()
+	if e, ok := os.LookupEnv("FSNOTIFY_BUFFER"); ok {
+		t.Logf("using FSNOTIFY_BUFFER=%v", e)
+		n, err2 := strconv.Atoi(e)
+		if err2 != nil {
+			t.Fatalf("FSNOTIFY_BUFFER: %v", err2)
+		}
+		w, err = NewBufferedWatcher(uint(n))
+	}
 	if err != nil {
 		t.Fatalf("newWatcher: %s", err)
 	}

mkdoc.zsh

@@ -72,16 +72,18 @@ EOF
 new=$(<<EOF
 // NewWatcher creates a new Watcher.
 EOF
+)
 
 newbuffered=$(<<EOF
-// NewBufferedWatcher creates a new Watcher with an optionally buffered event channel.
-// For almost all use cases an unbuffered Watcher will perform better than buffered.
-// Most kernels have de-duplication logic which allows for less activity in userspace
-// and generally better performance.  However there may be some cases where a very
-// large buffers can enable an application to keep up with mass file rotations.
-// You will always be better off increasing the kernel buffers over adding a large
-// userspace buffer, but if you can't control the kernel buffer then a buffered
-// watcher is a reasonable option.  You probably want NewWatcher.
+// NewBufferedWatcher creates a new Watcher with a buffered event channel.
+//
+// For almost all use cases an unbuffered Watcher will perform better; most
+// kernels have de-duplication logic, which means less activity in userspace and
+// generally better performance. However there may be some cases where a very
+// large buffer can enable an application to keep up with a very large number of
+// events. You will always be better off increasing the kernel buffers over
+// adding a large userspace buffer, but if you can't control the kernel buffer
+// then a buffered watcher is a reasonable option.
 EOF
 )