summaryrefslogtreecommitdiff
path: root/crates/tor-circmgr
Commit message (Collapse)AuthorAgeFilesLines
...
* | | Expand some comments based on review from @diziet.Nick Mathewson2022-03-211-1/+4
| | |
* | | circmgr: When planning, only keep one error; log them all.Nick Mathewson2022-03-211-3/+6
| | |
* | | dirmgr: Note errors and inform the circmgr about them.Nick Mathewson2022-03-211-0/+11
| | | | | | | | | | | | | | | | | | Some error types indicate that the guard has failed as a dircache. We should treat these errors as signs to close the circuit, and to mark the guard as having failed.
* | | Expose more peer information from circuit build failuresNick Mathewson2022-03-212-9/+46
| | | | | | | | | | | | | | | | | | | | | | | | | | | We already have the ability to get peer information from ChanMgr errors, and therefore from any RetryErrors that contain ChanMgr errors. This commit adds optional peer information to tor-proto errors, and a function to expose whatever peer information is available.
* | | circmgr: Change API for using FallbackDirsNick Mathewson2022-03-213-6/+7
| | | | | | | | | | | | | | | | | | 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.
* | | circmgr: use AllGuardsDown to retry betterNick Mathewson2022-03-212-8/+36
|/ / | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | Alternative API for set_isolation_group().Nick Mathewson2022-03-171-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | Instead of requiring a `Box<dyn Isolation>`, it now takes either a `Box<dyn Isolation>`, or an arbitrary `T` that implements `Isolation`. This API still allows the user to pass in a `Box<dyn Isolation>` if that's what they have, but it doesn't require them to Box the isolation on their own. Part of #414.
* | Merge branch 'test-isolation' into 'main'Nick Mathewson2022-03-175-86/+372
|\ \ | | | | | | | | | | | | new api for isolation See merge request tpo/core/arti!377
| * | replace TODOs with documentationtrinity-1686a2022-03-161-1/+1
| | |
| * | add trait to help test isolation related codetrinity-1686a2022-03-163-49/+182
| | |
| * | add tests on Isolation and fix conditional compilation issuestrinity-1686a2022-03-161-0/+58
| | | | | | | | | | | | | | | | | | it seems I added conditional compilation without noticing it?? and there was some errors when choosing a prefered runtime depending on feature flags
| * | refactor restrict_muttrinity-1686a2022-03-161-14/+15
| | |
| * | replace Arc with Box and use dyn-clonetrinity-1686a2022-03-162-43/+25
| | | | | | | | | | | | this also removes JoinResult
| * | add some documentation for new traitstrinity-1686a2022-03-161-12/+18
| | |
| * | replace isolated with compatibletrinity-1686a2022-03-161-13/+13
| | | | | | | | | | | | the inverted logic was too easy to mess up
| * | use downcast-rs instead of our own AsAnytrinity-1686a2022-03-162-15/+4
| | |
| * | fix existing teststrinity-1686a2022-03-162-68/+62
| | |
| * | add a join() on Isolationtrinity-1686a2022-03-161-4/+65
| | |
| * | testing new api for isolationTrinity Pointard2022-03-162-9/+71
| | |
* | | Merge branch 'config-partials-transparent' into 'main'Ian Jackson2022-03-172-51/+48
|\ \ \ | |/ / |/| | | | | | | | Absolish builders for CircMgrConfig and DirMgrConfig See merge request tpo/core/arti!417
| * | Add rationale for CircMgrConfig transparency and traitIan Jackson2022-03-161-1/+22
| | |
| * | Provide define_accessor_trait and use it to generate CircMgrConfigIan Jackson2022-03-161-20/+12
| | |
| * | Make CircMgrConfig transparent (and make it a trait)Ian Jackson2022-03-162-49/+33
| | | | | | | | | | | | See commentary for the rationale.
* | | derive_builder: Switch to upstream 0.11Ian Jackson2022-03-162-5/+5
|/ / | | | | | | | | | | | | | | | | This has the different syntax for builder field attributes than what I originally proposed in my MR, and which therefore is in the pinned branch. My upstream MR for the field attributes feature was morged: https://github.com/colin-kiegel/rust-derive-builder/issues/239
* | Use new upstream humantime_serde_option featureIan Jackson2022-03-141-5/+4
| | | | | | | | Replace all uses of our copy of this code.
* | humantime: Update to humantime-serde 1.1.1Ian Jackson2022-03-141-1/+1
| | | | | | | | | | | | | | | | This has the humantime_serde::option module, which we have upstreamed and are about to switch to. The remaining dependency with version = "1" is going to be removed in a moment.
* | Drop remaining conversion from FooConfig to FooConfigBuilderIan Jackson2022-03-071-34/+0
| |
* | Derive Deserialize for derive-builder-generated config buildersIan Jackson2022-03-071-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | I used git-grep -P '\#\[serde\((?!default|deny_unknown)' to find places where I needed to add additional attributes on the builder method fields. This is currently a bit duplicative, but when #371 is completely done, the validated (non-builder) configs won't need to be Deserialize any more. This is part of #371 and #372.
* | Use git source for derive_builder for now, for attrs featureIan Jackson2022-03-071-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We are going to want to specify custom attributes on fields of the builder struct. This feature was missing from derive_builder. This commitid is the current head of my MR branch https://github.com/colin-kiegel/rust-derive-builder/pull/237 https://github.com/ijackson/rust-derive-builder/tree/builder-field-attrs Using the commitid prevents surprises if that branch is updated. We will require this newer version of derive_builder. The version will need to be bumped again later, assuming the upstream MR is merged and upstream do a release containing the needed changes.
* | Merge branch 'educe-traits' into 'main'Ian Jackson2022-03-042-12/+7
|\ \ | | | | | | | | | | | | Replace many manual trait impls with use of educe See merge request tpo/core/arti!375
| * | Move skip_fmt into tor-basic-utilsIan Jackson2022-03-042-2/+2
| | | | | | | | | | | | | | | | | | | | | Code motion and the minimal mechanical changes. As per https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/375#note_2783078
| * | Replace manual Debug impl with educe in tor-circmgrIan Jackson2022-03-022-12/+7
| | |
* | | Replace manual Debug impl with std derive in tor-circmgrIan Jackson2022-03-021-10/+3
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | Merge branch 'clippy-allow-arc-clone' into 'main'Nick Mathewson2022-03-011-1/+0
|\ \ | | | | | | | | | | | | Disable clippy::clone_on_ref_ptr See merge request tpo/core/arti!352
| * | Disable clippy::clone_on_ref_ptrIan Jackson2022-02-241-1/+0
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This lint is IMO inherently ill-conceived. I have looked for the reasons why this might be thought to be a good idea and there were basically two (and they are sort of contradictory): I. "Calling ‘.clone()` on an Rc, Arc, or Weak can obscure the fact that only the pointer is being cloned, not the underlying data." This is the wording from https://rust-lang.github.io/rust-clippy/v0.0.212/#clone_on_ref_ptr It is a bit terse; we are left to infer why it is a bad idea to obscure this fact. It seems to me that if it is bad to obscure some fact, that must be because the fact is a hazard. But why would it be a hazard to not copy the underlying data ? In other languages, faliing to copy the underlying data is a serious correctness hazard. There is a whose class of bugs where things were not copied, and then mutated and/or reused in multiple places in ways that were not what the programmer intended. In my experience, this is a very common bug when writing Python and Javascript. I'm told it's common in golang too. But in Rust this bug is much much harder to write. The data inside an Arc is immutable. To have this bug you'd have use interior mutability - ie mess around with Mutex or RefCell. That provides a good barrier to these kind of accidents. II. "The reason for writing Rc::clone and Arc::clone [is] to make it clear that only the pointer is being cloned, as opposed to the underlying data. The former is always fast, while the latter can be very expensive depending on what is being cloned." This is the reasoning found here https://github.com/rust-lang/rust-clippy/issues/2048 This is saying that *not* using Arc::clone is hazardous. Specifically, that a deep clone is a performance hazard. But for this argument, the lint is precisely backwards. It's linting the "good" case and asking for it to be written in a more explicit way; while the supposedly bad case can be written conveniently. Also, many objects (in our codebase, and in all the libraries we use) that are Clone are in fact simply handles. They contain Arc(s) (or similar) and are cheap to clone. Indeed, that is the usual case. It does not make sense to distinguish in the syntax we use to clone such a handle, whether the handle is a transparent Arc, or an opaque struct containing one or more other handles. Forcing Arc::clone to be written as such makes for code churn when a type is changed from Arc<Something> to Something: Clone, or vice versa.
* | Bump all crates to 0.1.0arti-v0.1.0Nick Mathewson2022-03-011-18/+18
| |
* | resolve commenttrinity-1686a2022-02-281-1/+6
| |
* | add some error to retry_error instead of dropping ittrinity-1686a2022-02-281-45/+45
| |
* | Merge branch 'teardown' into 'main'Nick Mathewson2022-02-281-1/+12
|\ \ | | | | | | | | | | | | | | | | | | tor-circmgr: take_action: Handle Cancelled from the oneshot Closes #365 See merge request tpo/core/arti!363
| * | Add a debug! log message for source cancellationIan Jackson2022-02-281-0/+4
| | |
| * | Fix rustfmtIan Jackson2022-02-281-1/+1
| | |
| * | tor-circmgr: take_action: Handle Cancelled from the oneshotIan Jackson2022-02-251-1/+8
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>`.
* | Merge branch 'fix/210' into 'main'Nick Mathewson2022-02-281-1/+1
|\ \ | | | | | | | | | | | | | | | | | | don't return already errored pending circuit when searching new circuit matching spec Closes #210 See merge request tpo/core/arti!366
| * | don't return already errored pending circuit when searching new circuit ↵trinity-1686a2022-02-271-1/+1
| |/ | | | | | | matching spec
* | Fix two typosNick Mathewson2022-02-282-2/+2
| |
* | impl Debug for various internal typesIan Jackson2022-02-253-4/+25
|/ | | | | | | | 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.
* Merge remote-tracking branch 'origin/mr/340'Nick Mathewson2022-02-233-11/+21
|\
| * Make NoLock into BadApiUsage.Nick Mathewson2022-02-222-10/+14
| | | | | | | | | | | | To implement this, we had to refactor the tor_circmgr api for flushing state changes to disk, so that it checks if it has the lock, and only then tries to store.
| * Fold EK::Canceled into TransientFailureNick Mathewson2022-02-221-1/+7
| | | | | | | | Also add some TODO comments in circmgr for future work.
* | Merge branch 'restore_needless_borrow_check' into 'main'eta2022-02-231-1/+0
|\ \ | |/ |/| | | | | | | | | Remove clippy::needless_borrow exception in CI. Closes #310 See merge request tpo/core/arti!338
generated by cgit v1.2.3 (git 2.46.0) at 2026-08-28 19:19:57 +0000