fix(ot): only request a flush for outstanding RCOT allocations - #447
Open
mpzFixApplier wants to merge 1 commit into
Open
fix(ot): only request a flush for outstanding RCOT allocations#447mpzFixApplier wants to merge 1 commit into
mpzFixApplier wants to merge 1 commit into
Conversation
`SharedRCOTSender` and `SharedRCOTReceiver` derived `wants_flush()` from whether the shared buffer map was non-empty. A buffer stays in that map after a flush has filled it, until its owning instance picks the keys or OTs up, so during that window every instance sees pending work which does not exist. Flushing is collective: the barrier only releases once every live instance has arrived. An instance which re-entered `flush()` in that window joined the barrier alone, while the instances that had already drained their buffers reported `wants_flush() == false` and never joined again, hanging the session. This is timing dependent and gets likelier the more allocations a workload makes. Track outstanding allocations explicitly: `Buffer::count` is now cleared as soon as a flush fulfills it, rather than implicitly by removing the buffer once its owner drains it, and `wants_flush()` looks at those counts instead of at the presence of a buffer. A `flushing` flag keeps the decision to participate consistent, so an instance whose allocation was fulfilled after another instance already committed to a flush still joins the barrier. Draining a buffer no longer discards an allocation which was made while the flush was in progress.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
SharedRCOTSenderandSharedRCOTReceiverderivewants_flush()from whether the shared buffer map is non-empty. A buffer stays in that map after a flush has filled it, until its owning instance picks its keys/OTs up, so for that window every instance reports pending work which does not exist.Flushing is collective: the
AdaptiveBarrieronly releases once every live instance has arrived. An instance which re-entersflush()inside that window joins the barrier alone, while the instances that already drained their buffers reportwants_flush() == falseand never join again. The session then makes no further progress; with a timeout on the caller's side it surfaces asinsufficient RCOTs setup.It is timing dependent and gets likelier the more allocations a workload makes. In our testing it showed up in MPC-TLS sessions at sent-data budgets in the 6–8 KB range, roughly 1 in 20–40 sessions; at budgets around 1 KB we never saw it. It only reproduces through a downstream integration driving many allocations, not in mpz's own tests, which is why the regression coverage here is unit level.
Fix
Buffer::countnow tracks outstanding allocations: it is cleared as soon as a flush fulfills it, rather than implicitly when its owner later drains the buffer.wants_flush()looks at those counts instead of at the presence of a buffer.flushingflag keeps the decision to participate consistent across instances, so an instance whose allocation was fulfilled after another instance already committed to a flush still joins the barrier. It is cleared while all instances are parked at the barrier, which also covers the error paths.Sender and receiver had the same shape of bug and get the same treatment.
Tests
Unit tests on both sides: a fulfilled-but-not-yet-drained buffer must not keep asking for flushes, and entering a flush must be collective once one instance has committed to it.
cargo test --workspacepasses.Changelog
Added under
## [Unreleased]→### Fixed.