From 200d92121f797176286139473fd6eba06bcdc7fc Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Thu, 6 Jun 2024 14:09:35 -0400 Subject: [PATCH] types/lazy: add Peek method to SyncValue This adds the ability to "peek" at the value of a SyncValue, so that it's possible to observe a value without computing this. Updates tailscale/corp#17122 Signed-off-by: Andrew Dunham Co-authored-by: Brad Fitzpatrick Change-Id: I06f88c22a1f7ffcbc7ff82946335356bb0ef4622 --- cmd/stund/depaware.txt | 2 +- types/lazy/lazy.go | 78 ++++++++++++++++++++++++++++++++++++++--- types/lazy/sync_test.go | 41 ++++++++++++++++++++++ 3 files changed, 115 insertions(+), 6 deletions(-) diff --git a/cmd/stund/depaware.txt b/cmd/stund/depaware.txt index 7395e0f38..d1528c7ac 100644 --- a/cmd/stund/depaware.txt +++ b/cmd/stund/depaware.txt @@ -59,7 +59,7 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar tailscale.com/types/lazy from tailscale.com/version+ tailscale.com/types/logger from tailscale.com/tsweb tailscale.com/types/opt from tailscale.com/envknob+ - tailscale.com/types/ptr from tailscale.com/tailcfg + tailscale.com/types/ptr from tailscale.com/tailcfg+ tailscale.com/types/structs from tailscale.com/tailcfg+ tailscale.com/types/tkatype from tailscale.com/tailcfg+ tailscale.com/types/views from tailscale.com/net/tsaddr+ diff --git a/types/lazy/lazy.go b/types/lazy/lazy.go index 04629bcbe..755b5ca6f 100644 --- a/types/lazy/lazy.go +++ b/types/lazy/lazy.go @@ -4,7 +4,16 @@ // Package lazy provides types for lazily initialized values. package lazy -import "sync" +import ( + "sync" + "sync/atomic" + + "tailscale.com/types/ptr" +) + +// nilErrPtr is a sentinel *error value for SyncValue.err to signal +// that SyncValue.v is valid. +var nilErrPtr = ptr.To[error](nil) // SyncValue is a lazily computed value. // @@ -17,7 +26,17 @@ import "sync" type SyncValue[T any] struct { once sync.Once v T - err error + + // err is either: + // * nil, if not yet computed + // * nilErrPtr, if completed and nil + // * non-nil and not nilErrPtr on error. + // + // It is an atomic.Pointer so it can be read outside of the sync.Once.Do. + // + // Writes to err must happen after a write to v so a caller seeing a non-nil + // err can safely read v. + err atomic.Pointer[error] } // Set attempts to set z's value to val, and reports whether it succeeded. @@ -26,6 +45,7 @@ func (z *SyncValue[T]) Set(val T) bool { var wasSet bool z.once.Do(func() { z.v = val + z.err.Store(nilErrPtr) // after write to z.v; see docs wasSet = true }) return wasSet @@ -41,15 +61,63 @@ func (z *SyncValue[T]) MustSet(val T) { // Get returns z's value, calling fill to compute it if necessary. // f is called at most once. func (z *SyncValue[T]) Get(fill func() T) T { - z.once.Do(func() { z.v = fill() }) + z.once.Do(func() { + z.v = fill() + z.err.Store(nilErrPtr) // after write to z.v; see docs + }) return z.v } // GetErr returns z's value, calling fill to compute it if necessary. // f is called at most once, and z remembers both of fill's outputs. func (z *SyncValue[T]) GetErr(fill func() (T, error)) (T, error) { - z.once.Do(func() { z.v, z.err = fill() }) - return z.v, z.err + z.once.Do(func() { + var err error + z.v, err = fill() + + // Update z.err after z.v; see field docs. + if err != nil { + z.err.Store(ptr.To(err)) + } else { + z.err.Store(nilErrPtr) + } + }) + return z.v, *z.err.Load() +} + +// Peek returns z's value and a boolean indicating whether the value has been +// set successfully. If a value has not been set, the zero value of T is +// returned. +// +// This function is safe to call concurrently with Get/GetErr/Set, but it's +// undefined whether a value set by a concurrent call will be visible to Peek. +// +// To get any error that's been set, use PeekErr. +// +// If GetErr's fill function returned a valid T and an non-nil error, Peek +// discards that valid T value. PeekErr returns both. +func (z *SyncValue[T]) Peek() (v T, ok bool) { + if z.err.Load() == nilErrPtr { + return z.v, true + } + var zero T + return zero, false +} + +// PeekErr returns z's value and error and a boolean indicating whether the +// value or error has been set. If ok is false, T and err are the zero value. +// +// This function is safe to call concurrently with Get/GetErr/Set, but it's +// undefined whether a value set by a concurrent call will be visible to Peek. +// +// Unlike Peek, PeekErr reports ok if either v or err has been set, not just v, +// and returns both the T and err returned by GetErr's fill function. +func (z *SyncValue[T]) PeekErr() (v T, err error, ok bool) { + if e := z.err.Load(); e != nil { + return z.v, *e, true + } + var zero T + return zero, nil, false } // SyncFunc wraps a function to make it lazy. diff --git a/types/lazy/sync_test.go b/types/lazy/sync_test.go index d7bc913f7..ab3ed427d 100644 --- a/types/lazy/sync_test.go +++ b/types/lazy/sync_test.go @@ -5,6 +5,7 @@ package lazy import ( "errors" + "fmt" "sync" "testing" ) @@ -16,6 +17,11 @@ func TestSyncValue(t *testing.T) { if got != 42 { t.Fatalf("got %v; want 42", got) } + if p, ok := lt.Peek(); !ok { + t.Fatalf("Peek failed") + } else if p != 42 { + t.Fatalf("Peek got %v; want 42", p) + } })) if n != 0 { t.Errorf("allocs = %v; want 0", n) @@ -45,6 +51,12 @@ func TestSyncValueErr(t *testing.T) { if got != 0 || err != wantErr { t.Fatalf("got %v, %v; want 0, %v", got, err, wantErr) } + + if p, ok := lt.Peek(); !ok { + t.Fatalf("Peek failed") + } else if got != 0 { + t.Fatalf("Peek got %v; want 0", p) + } })) if n != 0 { t.Errorf("allocs = %v; want 0", n) @@ -59,6 +71,11 @@ func TestSyncValueSet(t *testing.T) { if lt.Set(43) { t.Fatalf("Set succeeded after first Set") } + if p, ok := lt.Peek(); !ok { + t.Fatalf("Peek failed") + } else if p != 42 { + t.Fatalf("Peek got %v; want 42", p) + } n := int(testing.AllocsPerRun(1000, func() { got := lt.Get(fortyTwo) if got != 42 { @@ -81,6 +98,30 @@ func TestSyncValueMustSet(t *testing.T) { lt.MustSet(43) } +func TestSyncValueErrPeek(t *testing.T) { + var sv SyncValue[int] + sv.GetErr(func() (int, error) { + return 123, errors.New("boom") + }) + p, ok := sv.Peek() + if ok { + t.Error("unexpected Peek success") + } + if p != 0 { + t.Fatalf("Peek got %v; want 0", p) + } + p, err, ok := sv.PeekErr() + if !ok { + t.Errorf("PeekErr ok=false; want true on error") + } + if got, want := fmt.Sprint(err), "boom"; got != want { + t.Errorf("PeekErr error=%v; want %v", got, want) + } + if p != 123 { + t.Fatalf("PeekErr got %v; want 123", p) + } +} + func TestSyncValueConcurrent(t *testing.T) { var ( lt SyncValue[int]