Skip to content

Commit 79f9bfd

Browse files
committed
fix wait.PollUntilContextCancel immediately executes condition once
1 parent 4734021 commit 79f9bfd

File tree

1 file changed

+18
-20
lines changed
  • staging/src/k8s.io/apimachinery/pkg/util/wait

1 file changed

+18
-20
lines changed

staging/src/k8s.io/apimachinery/pkg/util/wait/loop.go

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ func loopConditionUntilContext(ctx context.Context, t Timer, immediate, sliding
4040
var timeCh <-chan time.Time
4141
doneCh := ctx.Done()
4242

43+
if !sliding {
44+
timeCh = t.C()
45+
}
46+
4347
// if immediate is true the condition is
4448
// guaranteed to be executed at least once,
4549
// if we haven't requested immediate execution, delay once
@@ -50,17 +54,27 @@ func loopConditionUntilContext(ctx context.Context, t Timer, immediate, sliding
5054
}(); err != nil || ok {
5155
return err
5256
}
53-
} else {
57+
}
58+
59+
if sliding {
5460
timeCh = t.C()
61+
}
62+
63+
for {
64+
65+
// Wait for either the context to be cancelled or the next invocation be called
5566
select {
5667
case <-doneCh:
5768
return ctx.Err()
5869
case <-timeCh:
5970
}
60-
}
6171

62-
for {
63-
// checking ctx.Err() is slightly faster than checking a select
72+
// IMPORTANT: Because there is no channel priority selection in golang
73+
// it is possible for very short timers to "win" the race in the previous select
74+
// repeatedly even when the context has been canceled. We therefore must
75+
// explicitly check for context cancellation on every loop and exit if true to
76+
// guarantee that we don't invoke condition more than once after context has
77+
// been cancelled.
6478
if err := ctx.Err(); err != nil {
6579
return err
6680
}
@@ -77,21 +91,5 @@ func loopConditionUntilContext(ctx context.Context, t Timer, immediate, sliding
7791
if sliding {
7892
t.Next()
7993
}
80-
81-
if timeCh == nil {
82-
timeCh = t.C()
83-
}
84-
85-
// NOTE: b/c there is no priority selection in golang
86-
// it is possible for this to race, meaning we could
87-
// trigger t.C and doneCh, and t.C select falls through.
88-
// In order to mitigate we re-check doneCh at the beginning
89-
// of every loop to guarantee at-most one extra execution
90-
// of condition.
91-
select {
92-
case <-doneCh:
93-
return ctx.Err()
94-
case <-timeCh:
95-
}
9694
}
9795
}

0 commit comments

Comments
 (0)