| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
| |
Previously, if we had launch_parallelism > 1, and we were willing to
retry building a circuit max_retries times, then we'd launch up to
max_retries * launch_parallelism circuits before giving up. Ouch!
With this patch, we try to keep the total number of circuits
planned and attempted to the actual max_retries limit.
Part of #329; found with arti-testing.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
The guard manager is responsible for handing out the first hops of
tor circuits, keeping track of their successes and failures, and
remembering their states. Given that, it makes sense to store this
information here. It is not yet used; I'll be fixing that in
upcoming commits.
Arguably, this information no longer belongs in the directory
manager: I've added a todo about moving it.
This commit will break compilation on its own in a couple of places;
subsequent commits will fix it up.
|
| |
|
|
|
|
|
| |
This is the logical place for it, I think: the GuardMgr's job is to
pick the first hop for a circuit depending on remembered status for
possible first hops. Making this change will let us streamline the
code that interacts with these objects.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
| |
It'll soon more convenient to pass in FallbackDirs as a slice of
references, rather than just a slice of FallbackDirs: I'm going to
be changing how we handle these in tor-dirmgr.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
If all guards are down and they won't be retriable for a while, try
waiting that long to get whichever guard _is_ retriable.
Additionally, if we are making multiple circuit plans in parallel,
only report our planning as having failed if we failed at making
_all_ the plans. Previously we treated any failure as fatal for the
other plans, which could lead to trouble in the case when guards
were all down or pending.
Part of #407.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When I wrote this, I arranged to skip dumping the field `pending`.
This must have been because I thought that either
(a) PendingEntry couldn't `#[derive(Debug)]` (but it can)
and/or
(b) Some of the fields of PendingEntry ought not to be dumped because
they might contain (eg) packet data. But I think they don't: there's
just the spec, and the Result which is (basically) a Circ.
I tried preseving something closer to the original using educe, but
educe gets somehow tangled up with the generics, and the result fails
to compile. I haven't investigated this further.
|
| | |
|
| | |
|
| |\
| |
| |
| |
| |
| |
| | |
tor-circmgr: take_action: Handle Cancelled from the oneshot
Closes #365
See merge request tpo/core/arti!363
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Fixes #365
Inspection of the code and logs shows that:
* One of the plan futures' oneshots must be returning Cancelled
* This means that the corresponding sender must have been dropped
* The sender is owned by the task spawned by spawn_launch
Presumably that entire task gets dropped as part of executor shutdown,
or something.
The correct response in this situation is to declare that we are
shutting down, and stop trying to do stuff.
Unfortunately, despite trying quite hard by putting sleeps in various
strategic places, I have not been able to reproduce the problem. So I
can't be 100% sure that the new behaviour is correct.
But I am reasonably confident that this ought not to be able to occur
unless either 1. the task from spawn_launch is dropped, or 2. that
task somehow panics despite its attempts to trap panics and report
them as errors through the oneshot.
So this "burn it all down" action ought only to occur in actually
serious situations.
I observe that
3ff9b187ea26aaec4875067fcdbf485ecc9f597d
Handle panics from circuit construction.
changed the EK for PendingCanceled to EK::ReactorShuttingDown,
and there's From impl. I think, therefore, that it is right
to reuse this Error variant.
I don't quite understand why when take_action gets an actual error it
doesn't push it, but just logs it. But I am not changing that for
now.
Arguably the two instances of retry_error.push are a sign of an
inferior flow control pattern - maybe the loop body including the code
I am adding ought to be an IEFE returning
`Result<Option<circ>, crate::Error>`.
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | | |
don't return already errored pending circuit when searching new circuit matching spec
Closes #210
See merge request tpo/core/arti!366
|
| | |/
| |
| |
| | |
matching spec
|
| | | |
|
| |/
|
|
|
|
|
|
| |
I wanted this while debugging something.
The ad-hoc impl Debug with f.debug_struct is getting repetitive
and I've already perpetrated one copy-paste mistake.
We should consider using something like the `educe` crate's Clone.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
We handle them by reporting them to task that's waiting for the
circuit, then relaying the panic.
Doing so allows the waiting task to distinguish panics
(EK::Internal) from cases where the reactor dropped the task
entirely (EK::ReactorShuttingDown). And doing _that_ removes one
case of EK::Canceled, which helps us on our goals towards #348.
Closes #347.
|
| |
|
|
| |
This reduces our nesting, and will help us handle panics.
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| | |
From its old name, this error had implied that we were giving no
useful information when we were waiting on a pending cirucit request
that failed. In fact, this error would only happen if we dropped the
`mpsc::Sender` for a circuit attempt without reporting success or
failure.
|
| | |
| |
| |
| | |
There are a couple of tricky ones I'll do separately.
|
| | | |
|
| |/
|
|
|
|
|
|
|
|
| |
In one of the two places, nightly no longer warns. In the other
place, it's fine for nightly to warn: I just fixed the code to take
a slice instead.
Partial revert of 856aca879151c622512bc4b15c6307808fc83e82.
Resolves part of #310.
|
| |
|
|
|
|
|
|
| |
The type annotation may not be necessary for inference, but as a
comment it risks becoming false. So it should be uncommented, or
deleted.
Error types round here are not entirely trivial so uncomment it.
|
| |\
| |
| |
| |
| | |
Fix typos
See merge request tpo/core/arti!285
|
| | | |
|
| |/ |
|
| | |
|
| |
|
|
|
|
| |
See the new commentary text on `ClientCirc` for the rationale.
Signed-off-by: Ian Jackson <[email protected]>
|
| |
|
|
|
|
|
|
|
| |
We are going to get rid of the Arc. Happily there is an id which is
always constructed uniquely and preserved by clone.
(auto-deref lets us make the function take &Self instead of &Arc)
Signed-off-by: Ian Jackson <[email protected]>
|
| |
|
|
|
|
| |
This removes a lot of open-coded Arc::ptr_eq() calls
Signed-off-by: Ian Jackson <[email protected]>
|
| |
|
|
|
|
| |
This will make the code work when it's not an Arc any more.
Signed-off-by: Ian Jackson <[email protected]>
|
| |
|
|
|
|
|
| |
We are going to get rid of a lot of Arc, so we need the underlying
thing to be Clone.
Signed-off-by: Ian Jackson <[email protected]>
|
| |
|
|
|
|
| |
IIUC, these anticipatd a need to store min_exit_circs_per_port in
CircMgr. But the current design, where it goes into preemptive.rs and
thence to usage, seems to work fine.
|
| | |
|
| |
|
|
|
| |
Emphasize that circuit expiration functions _decide whether to
expire the circuit_, and don't expire it automatically.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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. ;)
|
| |
|
|
|
|
|
| |
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.
|
| | |
|
| |\ |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In preparation for making Arti build circuits preemptively, this commit
introduces `TargetCircUsage::Preemptive`, a circuit usage that works
somewhat differently from other ones: it requires at least 2 circuits to
exist that can exit the port it contains in order for an existing
circuit to match against it (path-spec.txt § 2.1.1); if that's not the
case, that usage will require building new circuits (in order that we
build enough to have 2 available).
This required refactoring how circuit reuse worked; now,
`CircList::find_open` uses the new `AbstractSpec::find_supported` trait
method, which we customize to implement the above check in the case of
`Preemptive` circuit usages. To make that work, `OpenEntry` now takes
two type parameters (the spec and circuit types), instead of taking a
builder type parameter and using its associated types. (We also got rid
of type constraints on that struct, yay!)
A WIP implementation of a preemptive circuit predictor that implements
path-spec.txt § 2.1.1 is also included, but this will require additional
effort to wire it up with the `CircMgr` properly.
|
| |\ \ |
|
| | |/ |
|
