Skip to content

fix(ot): only request a flush for outstanding RCOT allocations - #447

Open
mpzFixApplier wants to merge 1 commit into
ethereum:devfrom
mpzFixApplier:fix/shared-rcot-flush-race
Open

fix(ot): only request a flush for outstanding RCOT allocations#447
mpzFixApplier wants to merge 1 commit into
ethereum:devfrom
mpzFixApplier:fix/shared-rcot-flush-race

Conversation

@mpzFixApplier

Copy link
Copy Markdown

Problem

SharedRCOTSender and SharedRCOTReceiver derive wants_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 AdaptiveBarrier only releases once every live instance has arrived. An instance which re-enters flush() inside that window joins the barrier alone, while the instances that already drained their buffers report wants_flush() == false and never join again. The session then makes no further progress; with a timeout on the caller's side it surfaces as insufficient 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::count now 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.
  • A flushing flag 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.
  • Draining an instance's buffer no longer discards an allocation which was made while the flush was in progress.

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 --workspace passes.

Changelog

Added under ## [Unreleased]### Fixed.

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant