refactor: remove parking-lot dependency (#3034)

dignifiedquire · web-flow · commit 08671bb4dadc · 2024-12-12T11:58:12.000Z
## Description Currently there was an inconsistent usage of `std::sync::Mutex` and `parking_lot::Mutex`. This normalizes the usage to always use `std::sync::Mutex` and remove the external dependency. ## Breaking Changes  ## Notes & open questions  ## Change checklist - [x] Self-review. - [x] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [x] Tests if relevant. - [x] All breaking changes documented.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/iroh-dns-server/Cargo.toml b/iroh-dns-server/Cargo.toml
@@ -31,7 +31,6 @@ http = "1.0.0"
 humantime-serde = "1.1.1"
 iroh-metrics = { version = "0.29.0" }
 lru = "0.12.3"
-parking_lot = "0.12.1"
 pkarr = { version = "2.2.0", features = [ "async", "relay", "dht"], default-features = false }
 rcgen = "0.13"
 redb = "2.0.0"
diff --git a/iroh-net-report/src/lib.rs b/iroh-net-report/src/lib.rs
@@ -5,6 +5,10 @@
 //! etc and reachability to the configured relays.
 // Based on <https://github.com/tailscale/tailscale/blob/main/net/netcheck/netcheck.go>
 
+#![cfg_attr(iroh_docsrs, feature(doc_auto_cfg))]
+#![deny(missing_docs, rustdoc::broken_intra_doc_links)]
+#![cfg_attr(not(test), deny(clippy::unwrap_used))]
+
 use std::{
     collections::{BTreeMap, HashMap},
     fmt::{self, Debug},
diff --git a/iroh-net-report/src/ping.rs b/iroh-net-report/src/ping.rs
@@ -54,7 +54,7 @@ impl Pinger {
     fn get_client(&self, kind: ICMP) -> Result<Client> {
         let client = match kind {
             ICMP::V4 => {
-                let mut opt_client = self.0.client_v4.lock().unwrap();
+                let mut opt_client = self.0.client_v4.lock().expect("poisoned");
                 match *opt_client {
                     Some(ref client) => client.clone(),
                     None => {
@@ -66,7 +66,7 @@ impl Pinger {
                 }
             }
             ICMP::V6 => {
-                let mut opt_client = self.0.client_v6.lock().unwrap();
+                let mut opt_client = self.0.client_v6.lock().expect("poisoned");
                 match *opt_client {
                     Some(ref client) => client.clone(),
                     None => {
diff --git a/iroh-relay/Cargo.toml b/iroh-relay/Cargo.toml
@@ -43,7 +43,6 @@ iroh-metrics = { version = "0.29.0", default-features = false }
 libc = "0.2.139"
 num_enum = "0.7"
 once_cell = "1.18.0"
-parking_lot = "0.12.1"
 pin-project = "1"
 postcard = { version = "1", default-features = false, features = [
     "alloc",
diff --git a/iroh-relay/src/lib.rs b/iroh-relay/src/lib.rs
@@ -28,6 +28,7 @@
 
 #![cfg_attr(iroh_docsrs, feature(doc_auto_cfg))]
 #![deny(missing_docs, rustdoc::broken_intra_doc_links)]
+#![cfg_attr(not(test), deny(clippy::unwrap_used))]
 
 pub mod client;
 pub mod defaults;
diff --git a/iroh-relay/src/quic.rs b/iroh-relay/src/quic.rs
@@ -70,7 +70,8 @@ pub(crate) mod server {
                 vec![crate::quic::ALPN_QUIC_ADDR_DISC.to_vec()];
             let server_config = QuicServerConfig::try_from(quic_config.server_config)?;
             let mut server_config = quinn::ServerConfig::with_crypto(Arc::new(server_config));
-            let transport_config = Arc::get_mut(&mut server_config.transport).unwrap();
+            let transport_config =
+                Arc::get_mut(&mut server_config.transport).expect("not used yet");
             transport_config
                 .max_concurrent_uni_streams(0_u8.into())
                 .max_concurrent_bidi_streams(0_u8.into())
diff --git a/iroh/Cargo.toml b/iroh/Cargo.toml
@@ -51,7 +51,6 @@ netdev = "0.31.0"
 netwatch = { version = "0.2.0" }
 num_enum = "0.7"
 once_cell = "1.18.0"
-parking_lot = "0.12.1"
 pin-project = "1"
 pkarr = { version = "2", default-features = false, features = [
     "async",
diff --git a/iroh/src/discovery.rs b/iroh/src/discovery.rs
@@ -441,12 +441,11 @@ mod tests {
     use std::{
         collections::{BTreeSet, HashMap},
         net::SocketAddr,
-        sync::Arc,
+        sync::{Arc, Mutex},
         time::SystemTime,
     };
 
     use iroh_base::SecretKey;
-    use parking_lot::Mutex;
     use rand::Rng;
     use tokio_util::task::AbortOnDropHandle;
 
@@ -499,6 +498,7 @@ mod tests {
             self.shared
                 .nodes
                 .lock()
+                .unwrap()
                 .insert(self.node_id, (url.cloned(), addrs.clone(), now));
         }
 
@@ -508,7 +508,7 @@ mod tests {
             node_id: NodeId,
         ) -> Option<BoxStream<Result<DiscoveryItem>>> {
             let addr_info = match self.resolve_wrong {
-                false => self.shared.nodes.lock().get(&node_id).cloned(),
+                false => self.shared.nodes.lock().unwrap().get(&node_id).cloned(),
                 true => {
                     let ts = system_time_now() - 100_000;
                     let port: u16 = rand::thread_rng().gen_range(10_000..20_000);
diff --git a/iroh/src/discovery/pkarr/dht.rs b/iroh/src/discovery/pkarr/dht.rs
@@ -180,10 +180,11 @@ impl Builder {
 
     /// Builds the discovery mechanism.
     pub fn build(self) -> anyhow::Result<DhtDiscovery> {
-        let pkarr = self
-            .client
-            .unwrap_or_else(|| PkarrClient::new(Default::default()).unwrap())
-            .as_async();
+        let pkarr = match self.client {
+            Some(client) => client,
+            None => PkarrClient::new(Default::default())?,
+        };
+        let pkarr = pkarr.as_async();
         let ttl = self.ttl.unwrap_or(DEFAULT_PKARR_TTL);
         let relay_url = self.pkarr_relay;
         let dht = self.dht;
@@ -259,8 +260,8 @@ impl DhtDiscovery {
             let relay_publish = async {
                 if let Some(relay) = this.0.pkarr_relay.as_ref() {
                     tracing::info!(
-                        "publishing to relay: {}",
-                        this.0.relay_url.as_ref().unwrap().to_string()
+                        "publishing to relay: {:?}",
+                        this.0.relay_url.as_ref().map(|r| r.to_string())
                     );
                     match relay.publish(&signed_packet).await {
                         Ok(_) => {
@@ -286,8 +287,11 @@ impl DhtDiscovery {
         let Some(relay) = &self.0.pkarr_relay else {
             return;
         };
-        let url = self.0.relay_url.as_ref().unwrap();
-        tracing::info!("resolving {} from relay {}", pkarr_public_key.to_z32(), url);
+        tracing::info!(
+            "resolving {} from relay {:?}",
+            pkarr_public_key.to_z32(),
+            self.0.relay_url
+        );
         let response = relay.resolve(&pkarr_public_key).await;
         match response {
             Ok(Some(signed_packet)) => {
@@ -382,7 +386,7 @@ impl Discovery for DhtDiscovery {
         };
         let this = self.clone();
         let curr = tokio::spawn(this.publish_loop(keypair.clone(), signed_packet));
-        let mut task = self.0.task.lock().unwrap();
+        let mut task = self.0.task.lock().expect("poisoned");
         *task = Some(AbortOnDropHandle::new(curr));
     }
 
diff --git a/iroh/src/discovery/static_provider.rs b/iroh/src/discovery/static_provider.rs
@@ -74,7 +74,7 @@ impl StaticProvider {
     pub fn set_node_addr(&self, info: impl Into<NodeAddr>) -> Option<NodeAddr> {
         let last_updated = SystemTime::now();
         let info: NodeAddr = info.into();
-        let mut guard = self.nodes.write().unwrap();
+        let mut guard = self.nodes.write().expect("poisoned");
         let previous = guard.insert(
             info.node_id,
             NodeInfo {
@@ -96,7 +96,7 @@ impl StaticProvider {
     pub fn add_node_addr(&self, info: impl Into<NodeAddr>) {
         let info: NodeAddr = info.into();
         let last_updated = SystemTime::now();
-        let mut guard = self.nodes.write().unwrap();
+        let mut guard = self.nodes.write().expect("poisoned");
         match guard.entry(info.node_id) {
             Entry::Occupied(mut entry) => {
                 let existing = entry.get_mut();
@@ -116,7 +116,7 @@ impl StaticProvider {
 
     /// Get node info for the given node id.
     pub fn get_node_addr(&self, node_id: NodeId) -> Option<NodeAddr> {
-        let guard = self.nodes.read().unwrap();
+        let guard = self.nodes.read().expect("poisoned");
         let info = guard.get(&node_id)?;
         Some(NodeAddr {
             node_id,
@@ -127,7 +127,7 @@ impl StaticProvider {
 
     /// Remove node info for the given node id.
     pub fn remove_node_addr(&self, node_id: NodeId) -> Option<NodeAddr> {
-        let mut guard = self.nodes.write().unwrap();
+        let mut guard = self.nodes.write().expect("poisoned");
         let info = guard.remove(&node_id)?;
         Some(NodeAddr {
             node_id,
@@ -145,7 +145,7 @@ impl Discovery for StaticProvider {
         _endpoint: crate::Endpoint,
         node_id: NodeId,
     ) -> Option<futures_lite::stream::Boxed<anyhow::Result<super::DiscoveryItem>>> {
-        let guard = self.nodes.read().unwrap();
+        let guard = self.nodes.read().expect("poisoned");
         let info = guard.get(&node_id);
         match info {
             Some(addr_info) => {
diff --git a/iroh/src/endpoint.rs b/iroh/src/endpoint.rs
@@ -144,7 +144,7 @@ impl Builder {
             .collect::<Vec<_>>();
         let discovery: Option<Box<dyn Discovery>> = match discovery.len() {
             0 => None,
-            1 => Some(discovery.into_iter().next().unwrap()),
+            1 => Some(discovery.into_iter().next().expect("checked length")),
             _ => Some(Box::new(ConcurrentDiscovery::from_services(discovery))),
         };
         let msock_opts = magicsock::Options {
@@ -313,12 +313,16 @@ impl Builder {
     pub fn discovery_dht(mut self) -> Self {
         use crate::discovery::pkarr::dht::DhtDiscovery;
         self.discovery.push(Box::new(|secret_key| {
-            Some(Box::new(
-                DhtDiscovery::builder()
-                    .secret_key(secret_key.clone())
-                    .build()
-                    .unwrap(),
-            ))
+            match DhtDiscovery::builder()
+                .secret_key(secret_key.clone())
+                .build()
+            {
+                Ok(discovery) => Some(Box::new(discovery)),
+                Err(err) => {
+                    tracing::error!("failed to build discovery: {:?}", err);
+                    None
+                }
+            }
         }));
         self
     }
diff --git a/iroh/src/lib.rs b/iroh/src/lib.rs
@@ -230,6 +230,7 @@
 
 #![recursion_limit = "256"]
 #![deny(missing_docs, rustdoc::broken_intra_doc_links)]
+#![cfg_attr(not(test), deny(clippy::unwrap_used))]
 #![cfg_attr(iroh_docsrs, feature(doc_auto_cfg))]
 
 mod disco;
diff --git a/iroh/src/magicsock.rs b/iroh/src/magicsock.rs
diff --git a/iroh/src/magicsock/node_map.rs b/iroh/src/magicsock/node_map.rs
diff --git a/iroh/src/magicsock/relay_actor.rs b/iroh/src/magicsock/relay_actor.rs
diff --git a/iroh/src/test_utils.rs b/iroh/src/test_utils.rs
diff --git a/iroh/src/tls/certificate.rs b/iroh/src/tls/certificate.rs
