Skip to content

tor-proto: Rework StreamReader into a futures::Stream (and rename it)

This MR changes StreamReader in a few ways:

  1. Renamed to StreamReceiver.

    In rust, I think the typical nomenclature is to use "receiver" for channels, and "reader" for byte streams. For example mpsc::Receiver for something that returns objects and AsyncRead for something that reads bytes. Since we also have a DataReader for reading bytes, and StreamReader returns relay cell messages, I think renaming this from StreamReader to StreamReceiver better describes what it is (it's not a "reader" in the typical Read/AsyncRead sense).

  2. Removed the round trip when sending SENDMEs.

    Previously when sending a stream SENDME, the StreamReader would send a message to the circuit reactor and wait for a reply (confirmation or an error) before unblocking the stream. Now it sends a SENDME to the reactor but doesn't wait for a reply. The pros/cons of this change are listed in the "Make `StreamTarget::send_sendme` non-async" commit message.

    @gabi-250 We had some discussion about the StreamReader and SENDMEs before in !2867 (comment 3175302). I didn't follow up with that right away since I wasn't sure what the best way to change it was. This MR changes things again, so let me know if you have any thoughts about these changes.

  3. Remove StreamReader::recv and make StreamReader implement futures::Stream.

    This allows us to make DataReader simpler as it no longer needs to store a boxed future each time the stream blocks. I think there's even more cleanup that we could do, but I'll leave that for a future MR.

    This is also something that makes implement XON/XOFF flow control simpler. With flow control, the DataReader needs to be able to tell if the stream is empty or not. When the stream is hidden in a BoxSyncFuture, there's no way for the DataReader to access the StreamReader to check if it's empty. By getting rid of this future, we will be able to directly access the StreamReader to check if it's empty or not.

The StreamReader/StreamReceiver type is public in the API, but is not actually accessible. As far as I can tell there is no way to construct it or access it.

Edited by opara

Merge request reports

Loading