fix: extremely rare race conditions in retry loop #7789
Conversation
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #7789 +/- ##
==========================================
- Coverage 95.16% 95.15% -0.01%
==========================================
Files 1278 1278
Lines 115102 115114 +12
==========================================
+ Hits 109536 109539 +3
- Misses 5566 5575 +9
Continue to review full report at Codecov.
|
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
| * | ||
| * - The initial value of `cancelled_` is false. | ||
| * - `Cancel()` is the only operation that changes `cancelled_`, and it holds | ||
| * the `mu_` mutex while doing son. |
| * starts, as it is their callbacks who start the next step. | ||
| * - `StartIteration()` is always sequenced-before calls to `SetPending()`. | ||
| * - `SetPending()` never sets the `pending_operation_` to the future for | ||
| * an operation that already completed. |
There was a problem hiding this comment.
s/that already/that has already/
| * - `StartIteration()` and `SetPending()` both lock the same mutex `mu_` as | ||
| * `Cancel()`. | ||
| * - It follows that if `Cancel()` is invoked, then the `true` value will be | ||
| * visible any future calls to `StartIteration()` or `SetPending()`. |
| bool cancelled_ = false; | ||
| bool done_ = false; | ||
| std::uint_fast32_t iteration_ = 0; |
There was a problem hiding this comment.
TIL.
Do you want: s/fast32/fast64/ to match State::iteration, SetPending ?
| * as both `StartAttempt()` and `StartBackoff()` return immediately in this | ||
| * case. | ||
| * - Note that if `cancelled_` is true, `StartIteration()` terminates the | ||
| * retry loop by calling `SetDoneWithCancel()` if `cancelled_` is true. |
There was a problem hiding this comment.
s/`SetDoneWithCancel()` if `cancelled_` is true./`SetDoneWithCancel()`./
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Fixes #7788
I could not write a unit test that would reliably introduce the race condition.
This change is