This approach tries to preserve the current interface, but uses a
counter-based event backend to implement a coalescing stream of
events that can be represented as small integers. The advantage
here is that publishing events no longer needs to be a blocking
operation, since there is no queue to fill up.
(It's a protocol violation to get a SENDME when our send window is
already full.)
This patch makes SendWindow::put return a Result, so that it's
easier to do the right thing with it.
Closes#261.
arti!126 overhauled the `tor-proto` circuit reactor, but left out one
very important thing: actually decrementing the SENDME window for
streams (not circuits) when we send cells along them.
Since the circuit-level SENDME window would often prevent us from
running into a problem, this wasn't caught until my benchmarking efforts
noticed it (in the form of Tor nodes aborting the circuit for a protocol
violation).
fixes arti#260
We had no function to infallibly convert BoundedInt32<{0 or 1},H>
into a u32, even though we could have. Because of that, we were
treating weight_scale as an i32 when logically it's a u32 or a
NonZeroU32.
Moreover, it turns out we were using an incorrect minimum for the
bwweightscale param, which would in theory have allowed the
authorities to make us divide by zero.
This patch introduces the necessary From<> implementation and uses
it. It corrects the binimum bwweightscale, and prevents a
division-by-zero issue in case weight_scale is zero.
We don't want MutCfg to be automatially coneable, or we'll wind up with
surprises like the one that this patch fixes in TorClient.
(The "surprise" is that reconfigure() would only apply its
client-specific options to one client instance.)
If we allow overlapping reconfiguration requests, we introduce all
kinds of "fun" bugs. For example, we could wind up with a configuration
made up of parts of one reconfiguration attempt, and parts of another.
We can't change the authorities while in-flight: that would be pretty
miserable to implement.
Similarly we can't change the cache while in-flight.
Everything else should be fair game, though there are a couple of tricky
bits. I've tried to document those.
This required re-centralizing the configuration object for preemptive
circuits, since previously the settings from it were a bit spread out
over the crate.
And now the complexity begins: when the user changes the path_rules,
they not only want new circuits to obey those rules: they want
_all new requests_ to be put onto circuits that obey those rules.
That means that when the path rules become more restrictive, we need
to retire all the circuits, and make sure that currently pending
circuits aren't used for any requests.
If it's any comfort, doing this was even more complicated in C tor. ;)
It's useful to keep configuration objects inside a RwLock<Arc<>>, so we
can have slightly-stale pointers to the existing configuration structure
without holding locks too long.
This code adds a MutCfg type with basic support for this pattern,
and functions to make it a bit more ergonomic.
This patch doesn't actually make anything reconfigurable, but it
does create an API that will tell you "you can't change the value of
that!" If the API looks reasonable, I can start making it possible
to change the values of individual items.
`tor-rtcompat`'s `TlsConnector` trait previously included a method to
create a TLS-over-TCP connection, which implied creating a TCP stream
inside that method. This commit changes that, and makes the function
wrap a TCP stream, as returned from the runtime's `TcpProvider` trait
implementation, instead.
This means you can actually override `TcpProvider` and have it apply to
*all* connections Arti makes, which is useful for issues like arti#235
and other cases where you want to have a custom TCP stream
implementation.
This required updating the mock TCP/TLS types in `tor-rtmock` slightly;
due to the change in API, we now store whether a `LocalStream` should
actually be a TLS stream inside the stream itself, and check this
property on reads/writes in order to detect misuse. The fake TLS wrapper
checks this property and removes it in order to "wrap" the stream,
making reads and writes work again.
Previously we used Duration::mul_f64, which panics if its output is
out-of-range. That shouldn't actually be possible for the values
we're giving it, but probably it's better to just multiply in a safe
way.
This resolves a couple of XXXXs and therefore relates to #231.
The rand crate's documentation says it's not okay to rely on StdRng
having reproducible output. So instead, let's switch to ChaCha12Rng
instead (which is what StrRng currently uses).
We want to only use TODO in the codebase for non-blockers, and open
tickets for anything that is a bigger blocker than a TODO. These
XXXXs seem like definite non-blockers to me.
Part of arti#231.
Just as `in_same_family` is a member of Relay, so the function for
getting all the real family members of a relay should belong in the
same crate.
This change also removes the `family()` accessor: it gives the _claimed_ family rather
than the _acknlowedged_ family, and is therefore a bit dangerous.
There's still a hole in this logic; I've noted it in the Limitations
section. If we get a microdescriptor for a relay in between creating
and using the guard restriction, it might be omitted from the family
list.
There's not much reason to use a HashSet here, since we're just
going over the whole list.
This reverts commit 16e8489abb and does a little more
refactoring.
The redundant method was a `to_owned` that probably shouldn't have
been called that. It was only used in one place.
The tests should get tor-linkspec's line coverage up above 90%.
This was a relic of the old, now-unused "caret_enum!" macro.
Removing it gets caret's coverage to 100%.
Yes, technically this is a semver breaker on caret.
For this one I just wrote some "are things completely broken" tests
for the rand_compat wrappers. These won't detect subtle biases in
the RNGs! They'll only let you know if the wrappers have screwed up
in some way that always sets a given bit to 1 or 0.
This is mostly a finger exercise, and an experiment in "what does
grcov consider to be coverage". Here's what I've found out...
* In grcov's eyes, most #[derive(Foo)] lines count as containing code;
but calling any one derived function counts as calling those lines.
* Unlike with tarpaulin, it is actually possible to reach 100% grcov
line coverage. (Tarpaulin likes to pick "}" lines and tell you that
you never reached them; or sometimes it picks expression
statements that have the effect of a return, and tells you that
they're unreached. Even with these tests, tarpaulin claims that
the line coverage of tor-units is only 97.3%.)
* In rust, it may be a bit hopeless trying to get high function
coverage. Even though we've hit every line of the tor-units crate,
the function coverage from its own tests is only 9.38% (55.41%
from other crates). I think this is probably due to derived
functions, or maybe due to generics getting instantiated?
I've got no idea; the denominator for the function coverage
lines fluctuates oddly.
Nick Mathewson
Neel Chauhan
eta
Muhammad Falak R Wani
Daniel Eades
Trinity Pointard
dagon