Skip to content

Fix reactor ordering problems; refactor to use `SelectAll`

eta requested to merge eta/arti:eta/reactor-2-electric-boogaloo into main

A number of severe problems with the circuit reactor were fixed which could cause reordering of cells (which causes relays to terminate the circuit with a protocol violation, as they become unable to decrypt them). These mostly revolve around improper usage of queues:

  • The code assumed that a failure to place cells onto the channel would persist for the duration of a reactor cycle run. However, under high contention, this wouldn't always be the case.
    • This leads to some cells getting enqueued while others go straight through, before the enqueued cells.
    • To fix this, we block sending cells out of the channel while there are still some enqueued.
  • The hop-specific queues queued after encryption, not before. This was very brittle, and led to frequent mis-ordering.
    • This was fixed by making them not do that.

Additionally, the way we pull from streams has been significantly refactored; we now use SelectAll from the futures crate with a set of new StreamReceiver types that wrap the mpsc channel. This means we only poll channels that have actually woken up and need to be polled, among other improvements.

Doing this required re-sharing some of the SENDME windows in order to share them between the StreamReceiver and other parts of the code. This currently uses Arc instead of Rc because of the Send bound on futures spawned onto the runtime, even though it shouldn't really need to (since the types concerned are only ever shared between one thread).

Merge request reports