Sketch API for reconfiguration.

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.

Cargo.lock

@@ -2594,6 +2594,7 @@ dependencies = [
  "serde",
  "shellexpand",
  "thiserror",
+ "tracing",
 ]
 
 [[package]]

crates/arti-client/src/client.rs

@@ -43,6 +43,8 @@ pub struct TorClient<R: Runtime> {
     circmgr: Arc<tor_circmgr::CircMgr<R>>,
     /// Directory manager for keeping our directory material up to date.
     dirmgr: Arc<tor_dirmgr::DirMgr<R>>,
+    /// Location on disk where we store persistent data.
+    statemgr: FsStateMgr,
     /// Client address configuration
     addrcfg: ClientAddrConfig,
     /// Client DNS configuration
@@ -197,7 +199,7 @@ impl<R: Runtime> TorClient<R> {
         runtime.spawn(update_persistent_state(
             runtime.clone(),
             Arc::downgrade(&circmgr),
-            statemgr,
+            statemgr.clone(),
         ))?;
 
         runtime.spawn(continually_launch_timeout_testing_circuits(
@@ -219,15 +221,63 @@ impl<R: Runtime> TorClient<R> {
             client_isolation,
             circmgr,
             dirmgr,
+            statemgr,
             addrcfg: addr_cfg,
             timeoutcfg: timeout_cfg,
         })
     }
 
-    /// Return a new isolated `TorClient` instance.
+    /// Change the configuration of this TorClient to `new_config`.
     ///
-    /// The two `TorClient`s will share some internal state, but their
-    /// streams will never share circuits with one another.
+    /// The `how` describes whether to perform an all-or-nothing
+    /// reconfiguration: either all of the configuration changes will be applied,
+    /// or none will. If you have disabled all-or-nothing changes, then only
+    /// fatal errors will be reported in this function's return value.
+    ///
+    /// This function applies its changes to **all** TorClient instances derived
+    /// from the same call to [`TorClient::bootstrap`]: even ones whose circuits are
+    /// isolated from this handle.
+    ///
+    /// # Limitations
+    ///
+    /// At present, no options can actually be reconfigured: this function is a lie!
+    /// It only exists to demonstrate its intended API.
+    pub fn reconfigure(
+        &self,
+        new_config: &TorClientConfig,
+        how: tor_config::Reconfigure,
+    ) -> Result<()> {
+        match how {
+            tor_config::Reconfigure::AllOrNothing => {
+                // We have to check before we make any changes.
+                self.reconfigure(new_config, tor_config::Reconfigure::CheckAllOrNothing)?;
+            }
+            tor_config::Reconfigure::CheckAllOrNothing => {}
+            tor_config::Reconfigure::WarnOnFailures => {}
+            _ => {}
+        }
+
+        let circ_cfg = new_config.get_circmgr_config()?;
+        let dir_cfg = new_config.get_dirmgr_config()?;
+        let state_cfg = new_config.storage.expand_state_dir()?;
+        let addr_cfg = &new_config.address_filter;
+
+        if state_cfg != self.statemgr.path() {
+            how.cannot_change("storage.state_dir")?;
+        }
+        if &self.addrcfg != addr_cfg {
+            how.cannot_change("address_filter.*")?;
+        }
+        self.circmgr.reconfigure(&circ_cfg, how)?;
+        self.dirmgr.reconfigure(&dir_cfg, how)?;
+
+        Ok(())
+    }
+
+    /// Return a new isolated `TorClient` handle.
+    ///
+    /// The two `TorClient`s will share internal state and configuration, but
+    /// their streams will never share circuits with one another.
     ///
     /// Use this function when you want separate parts of your program to
     /// each have a TorClient handle, but where you don't want their

crates/arti-client/src/err.rs

@@ -50,6 +50,10 @@ pub enum Error {
     /// Building configuration for the client failed.
     #[error("Configuration failed: {0}")]
     Configuration(#[from] tor_config::ConfigBuildError),
+
+    /// Unable to change configuration.
+    #[error("Reconfiguration failed: {0}")]
+    Reconfigure(#[from] tor_config::ReconfigureError),
 }
 
 impl From<TimeoutError> for Error {

crates/tor-circmgr/src/build.rs

@@ -299,6 +299,11 @@ impl<R: Runtime> CircuitBuilder<R> {
         }
     }
 
+    /// Return this builder's [`PathConfig`](crate::PathConfig).
+    pub(crate) fn path_config(&self) -> &crate::PathConfig {
+        &self.path_config
+    }
+
     /// Flush state to the state manager.
     pub(crate) fn save_state(&self) -> Result<()> {
         // TODO: someday we'll want to only do this if there is something
@@ -366,11 +371,6 @@ impl<R: Runtime> CircuitBuilder<R> {
             .await
     }
 
-    /// Return the path configuration used by this builder.
-    pub(crate) fn path_config(&self) -> &crate::PathConfig {
-        &self.path_config
-    }
-
     /// Return true if this builder is currently learning timeout info.
     pub(crate) fn learning_timeouts(&self) -> bool {
         self.builder.timeouts.learning_timeouts()

crates/tor-circmgr/src/lib.rs

@@ -212,6 +212,25 @@ impl<R: Runtime> CircMgr<R> {
         Ok(circmgr)
     }
 
+    /// Try to change our configuration settings to `new_config`.
+    ///
+    /// The actual behavior here will depend on the value of `how`.
+    pub fn reconfigure(
+        &self,
+        new_config: &CircMgrConfig,
+        how: tor_config::Reconfigure,
+    ) -> std::result::Result<(), tor_config::ReconfigureError> {
+        let path_rules = self.mgr.peek_builder().path_config();
+        let circuit_timing = self.mgr.circuit_timing();
+        if path_rules != &new_config.path_rules {
+            how.cannot_change("path_rules.*")?;
+        }
+        if circuit_timing != &new_config.circuit_timing {
+            how.cannot_change("circuit_timing.*")?;
+        }
+        Ok(())
+    }
+
     /// Reload state from the state manager.
     ///
     /// We only call this method if we _don't_ have the lock on the state

crates/tor-circmgr/src/mgr.rs

@@ -686,6 +686,13 @@ impl<B: AbstractCircBuilder + 'static, R: Runtime> AbstractCircMgr<B, R> {
         *u = p.into();
     }
 
+    /// Return this manager's [`CircuitTiming`].
+    pub(crate) fn circuit_timing(&self) -> &CircuitTiming {
+        &self.circuit_timing
+    }
+
+    ///
+
     /// Return a circuit suitable for use with a given `usage`,
     /// creating that circuit if necessary, and restricting it
     /// under the assumption that it will be used for that spec.

crates/tor-config/Cargo.toml

@@ -6,13 +6,13 @@ edition = "2018"
 license = "MIT OR Apache-2.0"
 homepage = "https://gitlab.torproject.org/tpo/core/arti/-/wikis/home"
 description = "Low-level configuration for the Arti Tor implementation"
-keywords = [ "tor", "arti" ]
-categories = [ "config" ]
-repository="https://gitlab.torproject.org/tpo/core/arti.git/"
+keywords = ["tor", "arti"]
+categories = ["config"]
+repository = "https://gitlab.torproject.org/tpo/core/arti.git/"
 
 [features]
-default = [ "expand-paths" ]
-expand-paths = [ "shellexpand", "directories" ]
+default = ["expand-paths"]
+expand-paths = ["shellexpand", "directories"]
 
 [dependencies]
 thiserror = "1.0.24"
@@ -20,6 +20,7 @@ derive_builder = "0.10.2"
 once_cell = "1.7.2"
 serde = { version = "1.0.124", features = ["derive"] }
 shellexpand = { version = "2.1.0", optional = true }
+tracing = "0.1.26"
 directories = { version = "4.0.1", optional = true }
 
 [dev-dependencies]

crates/tor-config/src/err.rs

@@ -1,4 +1,4 @@
-//! Declare an error type.
+//! Declare error types.
 
 /// An error related to an option passed to Arti via a configuration
 /// builder.
@@ -58,6 +58,18 @@ impl ConfigBuildError {
     }
 }
 
+/// An error caused when attempting to reconfigure an existing Arti client, or one of its modules.
+#[derive(Debug, Clone, thiserror::Error)]
+#[non_exhaustive]
+pub enum ReconfigureError {
+    /// Tried to change a field that cannot change on a running client.
+    #[error("Cannot change {field} on a running client.")]
+    CannotChange {
+        /// The field (or fields) that we tried to change.
+        field: String,
+    },
+}
+
 #[cfg(test)]
 mod test {
     #![allow(clippy::unwrap_used)]

crates/tor-config/src/lib.rs

@@ -41,5 +41,35 @@
 mod err;
 mod path;
 
-pub use err::ConfigBuildError;
+pub use err::{ConfigBuildError, ReconfigureError};
 pub use path::CfgPath;
+
+/// Rules for reconfiguring a running Arti instance.
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum Reconfigure {
+    /// Perform no reconfiguration unless we can guarantee that all changes will be successful.
+    AllOrNothing,
+    /// Try to reconfigure as much as possible; warn on fields that we cannot reconfigure.
+    WarnOnFailures,
+    /// Don't reconfigure anything: Only check whether we can guarantee that all changes will be successful.
+    CheckAllOrNothing,
+}
+
+impl Reconfigure {
+    /// Called when we see a disallowed attempt to change `field`: either give a ReconfigureError,
+    /// or warn and return `Ok(())`, depending on the value of `self`.
+    pub fn cannot_change<S: AsRef<str>>(self, field: S) -> Result<(), ReconfigureError> {
+        match self {
+            Reconfigure::AllOrNothing | Reconfigure::CheckAllOrNothing => {
+                Err(ReconfigureError::CannotChange {
+                    field: field.as_ref().to_owned(),
+                })
+            }
+            Reconfigure::WarnOnFailures => {
+                tracing::warn!("Cannot change field {} on a running client", field.as_ref());
+                Ok(())
+            }
+        }
+    }
+}

crates/tor-dirmgr/src/config.rs

@@ -230,6 +230,11 @@ impl DirMgrConfig {
         SqliteStore::from_path(&self.cache_path, readonly)
     }
 
+    /// Return the configured cache path.
+    pub(crate) fn cache_path(&self) -> &std::path::Path {
+        self.cache_path.as_ref()
+    }
+
     /// Return a slice of the configured authorities
     pub(crate) fn authorities(&self) -> &[Authority] {
         self.network_config.authorities()

crates/tor-dirmgr/src/lib.rs

@@ -401,6 +401,33 @@ impl<R: Runtime> DirMgr<R> {
             .ok_or(Error::NoDownloadSupport)
     }
 
+    /// Try to change our configuration to `new_config`.
+    ///
+    /// Actual behavior will depend on the value of `how`.
+    pub fn reconfigure(
+        &self,
+        new_config: &DirMgrConfig,
+        how: tor_config::Reconfigure,
+    ) -> std::result::Result<(), tor_config::ReconfigureError> {
+        if new_config.cache_path() != self.config.cache_path() {
+            how.cannot_change("storage.cache_path")?;
+        }
+        if new_config.authorities() != self.config.authorities() {
+            how.cannot_change("network.authorities")?;
+        }
+        if new_config.fallbacks() != self.config.fallbacks() {
+            how.cannot_change("network.fallback_caches")?;
+        }
+        if new_config.schedule() != self.config.schedule() {
+            how.cannot_change("download_schedule.*")?;
+        }
+        if new_config.override_net_params() != self.config.override_net_params() {
+            how.cannot_change("override_net_params.*")?;
+        }
+
+        Ok(())
+    }
+
     /// Try to make this a directory manager with read-write access to its
     /// storage.
     ///

crates/tor-persist/src/fs.rs

@@ -82,6 +82,10 @@ impl FsStateMgr {
             .statepath
             .join(sanitize_filename::sanitize(key) + ".json")
     }
+    /// Return the top-level directory for this storage manager.
+    pub fn path(&self) -> &Path {
+        self.inner.statepath.as_ref()
+    }
 }
 
 impl StateMgr for FsStateMgr {