Skip to content

Commit fda7bef

Browse files
authored
fix(shared): make SetOnce.WaitWithContext deterministic once value is set (#3241)
When both the Done channel and ctx.Done() are ready, select picks a case at random, so a wait entered with an already cancelled context could report ctx.Err() for work that had already completed, randomly turning finished work into a spurious failure under deadline pressure. Check the Done channel first and document the contract: a set result always wins; ctx.Err() is returned only when the context becomes done before the result is set. This is deliberate -- the context bounds the waiting, not the delivery of an already-existing result, and fail-fast callers can still check ctx.Err() themselves. Drop the stale caller-migration TODO (from #257): the choice between Wait and WaitWithContext is purely about cancellation, and blocking Wait remains correct on teardown paths where cancellation must not skip work.
1 parent fcf92fa commit fda7bef

2 files changed

Lines changed: 32 additions & 1 deletion

File tree

packages/shared/pkg/utils/set_once.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,21 @@ func (s *SetOnce[T]) Result() (T, error) {
101101
return s.res.value, s.res.err
102102
}
103103

104-
// WaitWithContext TODO: Use waitWithContext in all places instead of Wait.
104+
// WaitWithContext returns the value or error set by SetValue or SetError,
105+
// waiting until the result is set or ctx is done, whichever comes first.
106+
//
107+
// An already set result always wins: once SetValue or SetError has been
108+
// called, WaitWithContext returns that result even if ctx is already
109+
// cancelled. ctx.Err() is returned only when ctx becomes done before the
110+
// result is set.
105111
func (s *SetOnce[T]) WaitWithContext(ctx context.Context) (T, error) {
112+
// An already set result always wins over an already cancelled context.
113+
select {
114+
case <-s.Done:
115+
return s.Result()
116+
default:
117+
}
118+
106119
select {
107120
case <-s.Done:
108121
return s.Result()

packages/shared/pkg/utils/set_once_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,24 @@ func TestSetOnceWaitWithContextCanceled(t *testing.T) {
111111
wg.Wait()
112112
}
113113

114+
func TestSetOnceWaitWithContextCanceledValueSet(t *testing.T) {
115+
t.Parallel()
116+
setOnce := NewSetOnce[int]()
117+
118+
require.NoError(t, setOnce.SetValue(1))
119+
120+
ctx, cancel := context.WithCancel(t.Context())
121+
cancel()
122+
123+
// A set value must always win over a cancelled context. Repeat to catch
124+
// the random select choice when both channels are ready.
125+
for range 200 {
126+
value, err := setOnce.WaitWithContext(ctx)
127+
require.NoError(t, err)
128+
assert.Equal(t, 1, value)
129+
}
130+
}
131+
114132
func TestSetOnceSetResultConcurrent(t *testing.T) {
115133
t.Parallel()
116134
setOnce := NewSetOnce[int]()

0 commit comments

Comments
 (0)