fix(storage): enforce mutual exclusion between Close() and Finalize() in async writers - #16211
Conversation
product-auto-label
Bot
added
the
api: storage
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces validation checks in Finalize() and Close() for both buffered and resumed async writer connections to prevent invalid state transitions (such as calling Close() after Finalize(), or calling them multiple times), along with corresponding unit tests. The review feedback highlights critical race conditions in these checks because the lock is released before the futures are moved out, allowing concurrent calls to bypass the .valid() checks. It is recommended to use the existing finalize_ and close_ boolean flags instead, which are safely updated under the lock.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16211 +/- ##
========================================
Coverage 92.27% 92.27%
========================================
Files 2214 2214
Lines 206394 206519 +125
========================================
+ Hits 190443 190559 +116
- Misses 15951 15960 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… in async writers
0ff069d to
264f190
Compare
2 participants
This PR addresses feedback regarding the interaction between
Close()andFinalize()inAsyncWriterConnectionimplementations (#16163 (comment)).Previously, if
Finalize()was called followed byClose(),Close()would still append its payload, but theWriteLoopwould prioritizeFinalizeStepoverCloseStep. TheClose()future would hang indefinitely because theclosed_promise was never satisfied. This change enforces mutual exclusion between these two terminal operations, ensuring they fail fast.