| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
| |
Found by rustdoc.
|
| |
|
|
|
| |
Returning `CircuitClosed` isn't right here since the circuit may not
have closed.
|
| |
|
| |
Co-authored-by: carti-it <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As near as I can tell, there are only three ways that the sender
corresponding to this rx can be dropped:
- `StreamMap::terminate` because the stream object itself was
dropped. (But see #2323.)
- `StreamMap::close_stream` because an END message or similar
has been received. (But see #2322.)
- The `StreamMap` has been dropped.
The first two cases are already handled, and AFAICT the third can
only happen when the circuit hop closes. That makes
`CircuitClosed` the appropriate error here, not `StreamProto`.
Part of a fix for #2304.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The `stream` module is client-specific, for the most part, so I am
moving it under `client`. Later on, we will factor out the parts that
can be shared with the relay implementation.
Note: this is a breaking change as the deleted `stream` module was
`pub`. We could've kept the module and reexported from it the public
types from `tor_proto::client::stream`, but I think it's better to have
this `client` namespacing, because it makes the separation between the
client and relay parts clearer.
|
| | |
|
| |
|
|
|
|
| |
The implementation from `tunnel` is client-specific, so we are renaming
the module accordingly. The more generic parts will be pulled into a
separate module in a future commit.
|
| |
|
|
| |
This will later become `DataReader`.
|
| | |
|
| |
|
|
|
| |
XON/XOFF flow control will want to know how many data bytes are queued
on a stream, so the new types track that.
|
| |
|
|
|
|
|
|
|
|
|
| |
Having this be a `futures::Stream` makes it nicer to work with. For
example we are able to remove a boxed future from `DataReaderState`,
which should be better for performance and makes the code simpler.
As mentioned in a previous commit when this type was named
`StreamReader`, this 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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are some pros/cons to this change:
Pros:
1. The only remaining `await` in `StreamReceiver::recv` is for polling
the receiver, which means we can turn the `StreamReceiver` into a
`Stream` in a future commit.
2. We won't block the user from receiving messages while we wait for the
circuit reactor to receive our SENDME message and send it on the
outgoing channel.
3. The `StreamReceiver` doesn't really care if it can't send the SENDME.
There isn't anything it can do, the circuit hop can go away for
external reasons like a DESTROY message, and we still want to return
all queued messages to the user anyways.
Cons:
1. If the `StreamReceiver` sends a SENDME request to the circuit
reactor, and the circuit reactor fails to send the SENDME, there's no
good way for the reactor to communicate this back to the
`StreamReceiver`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
In rust, 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, I think renaming
this from `StreamReader` to `StreamReceiver` better describes what it is
(it's not a "reader" in the typical `Read`/`AsyncRead` sense).
This 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.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Move StreamTarget to the tunnel module and the circuit module.
From now on streams will be implemented on tunnels, not circuits.
This moves `StreamTarget` to the tunnel module. A future change will
replace `ClientCirc` with `ClientTunnel` inside `StreamTarget`.
This is mostly code motion, best reviewed with `--color-moved`.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
| |
It is official, congestion control is now used at this commit by the
circuit reactor making circuit/sendme.rs unused. Will be removed with
another commit.
Related #534
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
This will make it easier to change their types.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
No actual bug here, just technical debt:
For `SendWindow`s, our tag system already ensured that we rejected
any SENDME that didn't correspond to an appropriate drain. Still,
it doesn't hurt to check.
For `RecvWindow`s, it would have been a protocol violation if we
ever did this, but it makes sense to make it an internal error if we
try.
Part of #1383.
|
| |
|
|
|
| |
For consistency with the terminology proposed in
https://gitlab.torproject.org/tpo/core/torspec/-/issues/253
|
| |
|
|
|
| |
This closes #525, and ensures, at last, that we don't parse any
message that we wouldn't accept.
|
| |
|
|
| |
Thanks to rust-analyzer for making this simple.
|
| |
|
|
|
|
|
|
|
| |
My proximate motivation is that tls-api wants its inner streams to be
Debug. But in general, I agree with the Rust API Guidelines notion
that almost everything should be Debug.
I have gone for the "dump all the things" approach. A more nuanced
approach would be possible too.
|
| |
|
|
|
|
| |
This took some refactoring, and gave an opportunity to notice
a few error variants that weren't being used, or didn't mean
what they said on the tin.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, the reactor would use an `UnboundedSender` to send things to
the `RawCellStream`, in order that the reactor wouldn't block if you
failed to read from the latter. This is bad, though, since it means
people can just run us out of memory by sending lots of things.
To fix this, we make the new `StreamReader` type (which does the reading
parts from `RawCellStream`) keep track of the stream's receive window
and issue SENDMEs once *it* has consumed enough data to require it, thus
meaning that we shouldn't get sent enough data to fill the channel
between reactor and `StreamReader` (and, if we do, that's someone trying
to flood us, and we abort the circuit).
As hinted to above, the `RawCellStream` was removed and its reading
functionalities replaced by `StreamReader`; its writing functionalities
are handled by `StreamTarget` anyway, so we just give out one of those
for the write end. This now means we don't need any mutexes!
note: this commit introduces a known issue, arti#230
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather like e8e9699c3c239d6c30f9ad414f15d3bad6ec03fd ("Get rid of
tor-proto's ChannelImpl, and use the reactor more instead"), this
admittedly rather large commit refactors the way circuits in `tor-proto`
work, centralising all of the logic in one large nonblocking reactor
which other things send messages into and out of, instead of having a
bunch of `-Impl` types that are protected by mutexes.
Congestion control becomes a lot simpler with this refactor, since the
reactor can manage both stream- and circuit-level congestion control
unilaterally without having to share this information with consumers,
meaning we can get rid of some locks.
The way streams work also changes, in order to facilitate better
handling of backpressure / fairness between streams: each stream now has
a set of channels to send and receive messages over, instead of sending
relay cells directly onto the channel (now, the reactor pulls messages
off each stream in each map, and tries to avoid doing so if it won't be
able to forward them yet).
Additionally, a lot of "close this circuit / stream" messages aren't
required any more, since that state is simply indicated by one end of a
channel going away. This should make cleanup a lot less brittle.
Getting all of this to work involved writing a fair deal of intricate
nonblocking code in Reactor::run_once that tries very hard to be mindful
of making backpressure work correctly (and congestion control); the old
code could get away with having tasks .await on things, but the new
reactor can't really do this (as it'd lock the reactor up), so has to do
everything in a nonblocking manner.
|
|
|
This will cause some pain for now, but now is really the best time
to do this kind of thing.
|
