fix(storage): enforce mutual exclusion between Close() and Finalize() in async writers#16211
fix(storage): enforce mutual exclusion between Close() and Finalize() in async writers#16211kalragauri wants to merge 2 commits into
Conversation
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.23% 92.24%
========================================
Files 2265 2265
Lines 210205 210330 +125
========================================
+ Hits 193888 194021 +133
+ Misses 16317 16309 -8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… in async writers
0ff069d to
264f190
Compare
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.