Merge branch 'sw1tch/fix_reconfigure_deadlock' into 'main'

fixes deadlock in TorClient::reconfigure

See merge request tpo/core/arti!1432

crates/arti-client/src/client.rs

@@ -819,18 +819,37 @@ impl<R: Runtime> TorClient<R> {
         // safely let two threads change them at once.  If we did, then we'd
         // introduce time-of-check/time-of-use bugs in checking our configuration,
         // deciding how to change it, then applying the changes.
-        let _guard = self.reconfigure_lock.lock().expect("Poisoned lock");
+        let guard = self.reconfigure_lock.lock().expect("Poisoned lock");
 
         match how {
             tor_config::Reconfigure::AllOrNothing => {
                 // We have to check before we make any changes.
-                self.reconfigure(new_config, tor_config::Reconfigure::CheckAllOrNothing)?;
+                self.reconfigure_inner(
+                    new_config,
+                    tor_config::Reconfigure::CheckAllOrNothing,
+                    &guard,
+                )?;
             }
             tor_config::Reconfigure::CheckAllOrNothing => {}
             tor_config::Reconfigure::WarnOnFailures => {}
             _ => {}
         }
 
+        // Actually reconfigure
+        self.reconfigure_inner(new_config, how, &guard)?;
+
+        Ok(())
+    }
+
+    /// This is split out from `reconfigure` so we can do the all-or-nothing
+    /// check without recursion. the caller to this method must hold the
+    /// `reconfigure_lock`.
+    fn reconfigure_inner(
+        &self,
+        new_config: &TorClientConfig,
+        how: tor_config::Reconfigure,
+        _reconfigure_lock_guard: &std::sync::MutexGuard<'_, ()>,
+    ) -> crate::Result<()> {
         let dir_cfg = new_config.dir_mgr_config().map_err(wrap_err)?;
         let state_cfg = new_config.storage.expand_state_dir().map_err(wrap_err)?;
         let addr_cfg = &new_config.address_filter;
@@ -1365,6 +1384,8 @@ mod test {
     #![allow(clippy::useless_vec)]
     //! <!-- @@ end test lint list maintained by maint/add_warning @@ -->
 
+    use tor_config::Reconfigure;
+
     use super::*;
     use crate::config::TorClientConfigBuilder;
     use crate::{ErrorKind, HasKind};
@@ -1480,4 +1501,23 @@ mod test {
             _ => panic!("unexpected isolation: {:?}", observed.isolation),
         };
     }
+
+    #[test]
+    fn reconfigure_all_or_nothing() {
+        tor_rtcompat::test_with_one_runtime!(|rt| async {
+            let state_dir = tempfile::tempdir().unwrap();
+            let cache_dir = tempfile::tempdir().unwrap();
+            let cfg = TorClientConfigBuilder::from_directories(state_dir, cache_dir)
+                .build()
+                .unwrap();
+            let tor_client = TorClient::with_runtime(rt)
+                .config(cfg.clone())
+                .bootstrap_behavior(BootstrapBehavior::Manual)
+                .create_unbootstrapped()
+                .unwrap();
+            tor_client
+                .reconfigure(&cfg, Reconfigure::AllOrNothing)
+                .unwrap();
+        });
+    }
 }