From c07aa2cfeda8d682eebd1cf58b3d3ac03e584fe6 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 22 Apr 2024 21:05:54 -0700 Subject: [PATCH] syncs: fix flaky test by deleting the code it tested (Watch) Fixes #11766 Change-Id: Id5a875aab23eb1b48a57dc379d0cdd42412fd18b Signed-off-by: Brad Fitzpatrick --- syncs/watchdog.go | 97 ------------------------------------------ syncs/watchdog_test.go | 77 --------------------------------- 2 files changed, 174 deletions(-) delete mode 100644 syncs/watchdog.go delete mode 100644 syncs/watchdog_test.go diff --git a/syncs/watchdog.go b/syncs/watchdog.go deleted file mode 100644 index 6af07c53c..000000000 --- a/syncs/watchdog.go +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package syncs - -import ( - "context" - "sync" - "time" -) - -// Watch monitors mu for contention. -// On first call, and at every tick, Watch locks and unlocks mu. -// (Tick should be large to avoid adding contention to mu.) -// Max is the maximum length of time Watch will wait to acquire the lock. -// The time required to lock mu is sent on the returned channel. -// Watch exits when ctx is done, and closes the returned channel. -func Watch(ctx context.Context, mu sync.Locker, tick, max time.Duration) chan time.Duration { - // Set up the return channel. - c := make(chan time.Duration) - var ( - closemu sync.Mutex - closed bool - ) - sendc := func(d time.Duration) { - closemu.Lock() - defer closemu.Unlock() - if closed { - // Drop values written after c is closed. - return - } - select { - case c <- d: - case <-ctx.Done(): - } - } - closec := func() { - closemu.Lock() - defer closemu.Unlock() - close(c) - closed = true - } - - // check locks the mutex and writes how long it took to c. - // check returns ~immediately. - check := func() { - // Start a race between two goroutines. - // One locks the mutex; the other times out. - // Ensure that only one of the two gets to write its result. - // Since the common case is that locking the mutex is fast, - // let the timeout goroutine exit early when that happens. - var sendonce sync.Once - done := make(chan bool) - go func() { - start := time.Now() - mu.Lock() - mu.Unlock() - elapsed := time.Since(start) - if elapsed > max { - elapsed = max - } - close(done) - sendonce.Do(func() { sendc(elapsed) }) - }() - go func() { - select { - case <-time.After(max): - // the other goroutine may not have sent a value - sendonce.Do(func() { sendc(max) }) - case <-done: - // the other goroutine sent a value - } - }() - } - - // Check once at startup. - // This is mainly to make testing easier. - check() - - // Start the watchdog goroutine. - // It checks the mutex every tick, until ctx is done. - go func() { - ticker := time.NewTicker(tick) - for { - select { - case <-ctx.Done(): - closec() - ticker.Stop() - return - case <-ticker.C: - check() - } - } - }() - - return c -} diff --git a/syncs/watchdog_test.go b/syncs/watchdog_test.go deleted file mode 100644 index e85e6cd75..000000000 --- a/syncs/watchdog_test.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package syncs - -import ( - "context" - "runtime" - "sync" - "testing" - "time" -) - -// Time-based tests are fundamentally flaky. -// We use exaggerated durations in the hopes of minimizing such issues. - -func TestWatchUncontended(t *testing.T) { - mu := new(sync.Mutex) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - // Once an hour, and now, check whether we can lock mu in under an hour. - tick := time.Hour - max := time.Hour - c := Watch(ctx, mu, tick, max) - d := <-c - if d == max { - t.Errorf("uncontended mutex did not lock in under %v", max) - } -} - -func TestWatchContended(t *testing.T) { - mu := new(sync.Mutex) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - // Every hour, and now, check whether we can lock mu in under a millisecond, - // which is enough time for an uncontended mutex by several orders of magnitude. - tick := time.Hour - max := time.Millisecond - mu.Lock() - defer mu.Unlock() - c := Watch(ctx, mu, tick, max) - d := <-c - if d != max { - t.Errorf("contended mutex locked in under %v", max) - } -} - -func TestWatchMultipleValues(t *testing.T) { - mu := new(sync.Mutex) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() // not necessary, but keep vet happy - // Check the mutex every millisecond. - // The goal is to see that we get a sufficient number of values out of the channel. - tick := time.Millisecond - max := time.Millisecond - c := Watch(ctx, mu, tick, max) - start := time.Now() - n := 0 - for d := range c { - n++ - if d == max { - t.Errorf("uncontended mutex did not lock in under %v", max) - } - if n == 10 { - cancel() - } - } - // See https://github.com/golang/go/issues/44343 - on Windows the Go runtime timer resolution is currently too coarse. Allow longer in that case. - want := 100 * time.Millisecond - if runtime.GOOS == "windows" { - want = 500 * time.Millisecond - } - - if elapsed := time.Since(start); elapsed > want { - t.Errorf("expected 1 event per millisecond, got only %v events in %v", n, elapsed) - } -}