fix(storage): guard against overlapping stream requests in AsyncWriterConnectionBuffered - #16364
Conversation
…ncWriterConnectionBuffered
There was a problem hiding this comment.
Code Review
This pull request refactors the WriteLoop state machine in AsyncWriterConnectionBufferedState to manage the writing_ flag within individual execution branches, ensuring that the asynchronous pipeline is correctly marked as idle only when no further operations are pending. It also adds unit tests to verify that Finalize and Write operations are correctly queued and not executed concurrently while a Flush is in-flight. Feedback suggests simplifying WriteLoop by setting writing_ = true at the start of the function and only resetting it to false at the end if no asynchronous step is dispatched, which reduces duplicated code and aligns with the repository style guide.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16364 +/- ##
==========================================
- Coverage 92.26% 92.26% -0.01%
==========================================
Files 2237 2237
Lines 210434 210497 +63
==========================================
+ Hits 194166 194207 +41
- Misses 16268 16290 +22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
In writer_connection_buffered.cc, WriteLoop previously computed its active state using:
When an explicit
Flush()was called with an empty/0-byte payload,resend_buffer_had no unsent bytes, causingwriting_to evaluate to false. WhileFlushStep()was asynchronously executing on the gRPC stream, a subsequent operation (such asFinalize()) observedwriting_ == false, bypassing the concurrency guard and initiating a concurrent gRPC operation on the same stream. This stream collision corrupted the gRPC state machine, causing pending promises to stall until timeout and resulting in premature teardown.In this PR, WriteLoop has been updated to maintain
writing_ = trueacross all dispatched asynchronous steps (WriteStep, FinalizeStep, CloseStep, FlushStep), clearingwriting_ = falseonly when no operations are pending: