Move responsibility for GuardMgr NetDir updates to GuardMgr.

Previously it was the job of a task in CircMgr to do this; but we're
going to want to give GuardMgr full access to the latest NetDir for
this, and for other code-simplification reasons.

With this change I'm deprecating a couple of functions in
tor-circmgr.  It's no longer necessary for us to have an artificial
external way for you to feed new NetDirs to a circmgr.  (I could
just remove them, but I want practice deprecating.)

crates/arti-client/src/client.rs

@@ -469,13 +469,6 @@ impl<R: Runtime> TorClient<R> {
 
         self.dirmgr.bootstrap().await?;
 
-        self.circmgr.update_network_parameters(
-            self.dirmgr
-                .latest_netdir()
-                .ok_or(ErrorDetail::DirMgr(tor_dirmgr::Error::DirectoryNotPresent))?
-                .params(),
-        );
-
         // Since we succeeded, disarm the unlock guard.
         unlock_guard.disarm();
 

crates/tor-circmgr/src/lib.rs

@@ -275,6 +275,11 @@ impl<R: Runtime> CircMgr<R> {
             ))
             .map_err(|e| Error::from_spawn("preemptive circuit launcher", e))?;
 
+        self.mgr
+            .peek_builder()
+            .guardmgr()
+            .install_netdir_provider(&dir_provider.clone().upcast_arc())?;
+
         Ok(ret)
     }
 
@@ -353,7 +358,11 @@ impl<R: Runtime> CircMgr<R> {
     /// Reconfigure this circuit manager using the latest set of
     /// network parameters.
     ///
-    /// (NOTE: for now, this only affects circuit timeout estimation.)
+    /// This is deprecated as a public function: `launch_background_tasks` now
+    /// ensures that this happens as needed.
+    #[deprecated(
+        note = "There is no need to call this function if you have used launch_background_tasks"
+    )]
     pub fn update_network_parameters(&self, p: &tor_netdir::params::NetParameters) {
         self.mgr.update_network_parameters(p);
         self.mgr.peek_builder().update_network_parameters(p);
@@ -373,9 +382,10 @@ impl<R: Runtime> CircMgr<R> {
 
     /// Reconfigure this circuit manager using the latest network directory.
     ///
-    /// This should be called on _any_ change to the network, as opposed to
-    /// [`CircMgr::update_network_parameters`], which should only be
-    /// called when the parameters change.
+    /// This function is deprecated: `launch_background_tasks` now makes sure that this happens as needed.
+    #[deprecated(
+        note = "There is no need to call this function if you have used launch_background_tasks"
+    )]
     pub fn update_network(&self, netdir: &NetDir) {
         self.mgr.peek_builder().guardmgr().update_network(netdir);
     }
@@ -550,34 +560,16 @@ impl<R: Runtime> CircMgr<R> {
     {
         use DirEvent::*;
         while let Some(event) = events.next().await {
-            match event {
-                NewConsensus => {
-                    if let (Some(cm), Some(dm)) = (Weak::upgrade(&circmgr), Weak::upgrade(&dirmgr))
-                    {
-                        let netdir = dm
-                            .latest_netdir()
-                            .expect("got new consensus event, without a netdir?");
-                        cm.update_network_parameters(netdir.params());
-                        cm.update_network(&netdir);
-                    } else {
-                        debug!("Circmgr or dirmgr has disappeared; task exiting.");
-                        break;
-                    }
-                }
-                NewDescriptors => {
-                    if let (Some(cm), Some(dm)) = (Weak::upgrade(&circmgr), Weak::upgrade(&dirmgr))
-                    {
-                        let netdir = dm
-                            .latest_netdir()
-                            .expect("got new descriptors event, without a netdir?");
-                        cm.update_network(&netdir);
-                    } else {
-                        debug!("Circmgr or dirmgr has disappeared; task exiting.");
-                        break;
-                    }
-                }
-                _ => {
-                    // Nothing we recognize.
+            if matches!(event, NewConsensus) {
+                if let (Some(cm), Some(dm)) = (Weak::upgrade(&circmgr), Weak::upgrade(&dirmgr)) {
+                    let netdir = dm
+                        .latest_netdir()
+                        .expect("got new consensus event, without a netdir?");
+                    #[allow(deprecated)]
+                    cm.update_network_parameters(netdir.params());
+                } else {
+                    debug!("Circmgr or dirmgr has disappeared; task exiting.");
+                    break;
                 }
             }
         }

crates/tor-guardmgr/src/daemon.rs

@@ -87,3 +87,33 @@ pub(crate) async fn run_periodic<R: tor_rtcompat::SleepProvider>(
         runtime.sleep(delay).await;
     }
 }
+
+/// Background task to keep a guard manager up-to-date with a given network
+/// directory provider.
+pub(crate) async fn keep_netdir_updated<RT: tor_rtcompat::Runtime>(
+    runtime: RT,
+    inner: Weak<Mutex<GuardMgrInner>>,
+    netdir_provider: Weak<dyn tor_netdir::NetDirProvider>,
+) {
+    use tor_netdir::DirEvent;
+
+    let mut event_stream = match netdir_provider.upgrade().map(|p| p.events()) {
+        Some(s) => s,
+        None => return,
+    };
+
+    while let Some(event) = event_stream.next().await {
+        match event {
+            DirEvent::NewConsensus | DirEvent::NewDescriptors => {
+                if let (Some(inner), Some(provider)) = (inner.upgrade(), netdir_provider.upgrade())
+                {
+                    let mut inner = inner.lock().expect("Poisoned lock");
+                    if let Some(netdir) = provider.latest_netdir() {
+                        inner.update(runtime.wallclock(), Some(&netdir));
+                    }
+                }
+            }
+            _ => {}
+        }
+    }
+}

crates/tor-guardmgr/src/lib.rs

@@ -141,6 +141,7 @@ use std::collections::{HashMap, HashSet};
 use std::net::SocketAddr;
 use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant, SystemTime};
+use tor_netdir::NetDirProvider;
 use tor_proto::ClockSkew;
 use tracing::{debug, info, trace, warn};
 
@@ -335,6 +336,33 @@ impl<R: Runtime> GuardMgr<R> {
         Ok(GuardMgr { runtime, inner })
     }
 
+    /// Install a [`NetDirProvider`] for use by this guard manager.
+    ///
+    /// It will be used to keep the guards up-to-date with changes from the
+    /// network directory, and to find new guards when no NetDir is provided to
+    /// select_guard().
+    ///
+    /// TODO: The second part is not implemented yet.
+    ///
+    /// TODO: we should eventually return some kind of a task handle from this
+    /// task, even though it is not strictly speaking periodic.
+    pub fn install_netdir_provider(
+        &self,
+        provider: &Arc<dyn NetDirProvider>,
+    ) -> Result<(), GuardMgrError> {
+        let weak_provider = Arc::downgrade(provider);
+        let weak_inner = Arc::downgrade(&self.inner);
+        let rt_clone = self.runtime.clone();
+        self.runtime
+            .spawn(daemon::keep_netdir_updated(
+                rt_clone,
+                weak_inner,
+                weak_provider,
+            ))
+            .map_err(|e| GuardMgrError::from_spawn("periodic guard netdir updater", e))?;
+        Ok(())
+    }
+
     /// Flush our current guard state to the state manager, if there
     /// is any unsaved state.
     pub fn store_persistent_state(&self) -> Result<(), GuardMgrError> {