Skip to content

Commit eb383a6

Browse files
authored
fix(iroh): Use std Mutex instead of tokio Mutex (#3374)
## Description Per tokio docs, if a mutex guard is not held across await points, it is possible and preferable to use std mutex. ## Change checklist <!-- Remove any that are not relevant. --> - [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.
1 parent ac2a3f2 commit eb383a6

File tree

2 files changed

+19
-10
lines changed

2 files changed

+19
-10
lines changed

iroh/src/magicsock.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use std::{
2323
pin::Pin,
2424
sync::{
2525
atomic::{AtomicBool, AtomicU64, Ordering},
26-
Arc, RwLock,
26+
Arc, Mutex, RwLock,
2727
},
2828
task::{Context, Poll},
2929
};
@@ -47,7 +47,7 @@ use quinn::{AsyncUdpSocket, ServerConfig};
4747
use rand::Rng;
4848
use smallvec::SmallVec;
4949
use snafu::{ResultExt, Snafu};
50-
use tokio::sync::{mpsc, Mutex};
50+
use tokio::sync::{mpsc, Mutex as AsyncMutex};
5151
use tokio_util::sync::CancellationToken;
5252
use tracing::{
5353
debug, error, event, info, info_span, instrument, trace, trace_span, warn, Instrument, Level,
@@ -1089,7 +1089,7 @@ struct DirectAddrUpdateState {
10891089
#[cfg(not(wasm_browser))]
10901090
port_mapper: portmapper::Client,
10911091
/// The prober that discovers local network conditions, including the closest relay relay and NAT mappings.
1092-
net_reporter: Arc<Mutex<net_report::Client>>,
1092+
net_reporter: Arc<AsyncMutex<net_report::Client>>,
10931093
relay_map: RelayMap,
10941094
run_done: mpsc::Sender<()>,
10951095
}
@@ -1116,7 +1116,7 @@ impl DirectAddrUpdateState {
11161116
fn new(
11171117
msock: Arc<MagicSock>,
11181118
#[cfg(not(wasm_browser))] port_mapper: portmapper::Client,
1119-
net_reporter: Arc<Mutex<net_report::Client>>,
1119+
net_reporter: Arc<AsyncMutex<net_report::Client>>,
11201120
relay_map: RelayMap,
11211121
run_done: mpsc::Sender<()>,
11221122
) -> Self {
@@ -1385,7 +1385,7 @@ impl Handle {
13851385
msock.clone(),
13861386
#[cfg(not(wasm_browser))]
13871387
port_mapper,
1388-
Arc::new(Mutex::new(net_reporter)),
1388+
Arc::new(AsyncMutex::new(net_reporter)),
13891389
relay_map,
13901390
direct_addr_done_tx,
13911391
);
@@ -1461,7 +1461,9 @@ impl Handle {
14611461
self.msock.closing.store(true, Ordering::Relaxed);
14621462
self.actor_token.cancel();
14631463

1464-
if let Some(task) = self.actor_task.lock().await.take() {
1464+
// MutexGuard is not held across await points
1465+
let task = self.actor_task.lock().expect("poisoned").take();
1466+
if let Some(task) = task {
14651467
// give the tasks a moment to shutdown cleanly
14661468
let shutdown_done = time::timeout(Duration::from_millis(100), async move {
14671469
if let Err(err) = task.await {
@@ -1502,7 +1504,7 @@ struct DiscoState {
15021504
/// Encryption key for this node.
15031505
secret_encryption_key: crypto_box::SecretKey,
15041506
/// The state for an active DiscoKey.
1505-
secrets: std::sync::Mutex<HashMap<PublicKey, SharedSecret>>,
1507+
secrets: Mutex<HashMap<PublicKey, SharedSecret>>,
15061508
/// Disco (ping) queue
15071509
sender: mpsc::Sender<(SendAddr, PublicKey, disco::Message)>,
15081510
}

iroh/src/protocol.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,19 @@
3232
//! }
3333
//! }
3434
//! ```
35-
use std::{collections::BTreeMap, future::Future, pin::Pin, sync::Arc};
35+
use std::{
36+
collections::BTreeMap,
37+
future::Future,
38+
pin::Pin,
39+
sync::{Arc, Mutex},
40+
};
3641

3742
use iroh_base::NodeId;
3843
use n0_future::{
3944
join_all,
4045
task::{self, AbortOnDropHandle, JoinSet},
4146
};
4247
use snafu::{Backtrace, Snafu};
43-
use tokio::sync::Mutex;
4448
use tokio_util::sync::CancellationToken;
4549
use tracing::{error, info_span, trace, warn, Instrument};
4650

@@ -338,7 +342,10 @@ impl Router {
338342
self.cancel_token.cancel();
339343

340344
// Wait for the main task to terminate.
341-
if let Some(task) = self.task.lock().await.take() {
345+
346+
// MutexGuard is not held across await point
347+
let task = self.task.lock().expect("poisoned").take();
348+
if let Some(task) = task {
342349
task.await?;
343350
}
344351

0 commit comments

Comments
 (0)