From 73ea8e12fbb335200cd41e53da67a056e99f2c58 Mon Sep 17 00:00:00 2001 From: Luke Yue Date: Fri, 12 Dec 2025 23:02:19 +0800 Subject: [PATCH] fix(rdmacm): use `or_insert_with()` to avoid eager `DeviceContext` creation `or_insert()` eagerly evaluates the default argument even when the entry exists, causing a new `DeviceContext` to be created and immediately dropped. This triggers `ibv_close_device()` on the shared verbs context, corrupting the cached entry. Switch to `or_insert_with()` so the `DeviceContext` is only constructed when the `HashMap` entry is missing. Signed-off-by: Luke Yue --- src/rdmacm/communication_manager.rs | 49 ++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/src/rdmacm/communication_manager.rs b/src/rdmacm/communication_manager.rs index 920fadd..d938b3e 100644 --- a/src/rdmacm/communication_manager.rs +++ b/src/rdmacm/communication_manager.rs @@ -847,12 +847,12 @@ impl Identifier { } let mut guard = DEVICE_LISTS.lock().unwrap(); - let device_ctx = guard - .entry((*cm_id.as_ptr()).verbs as usize) - .or_insert(Arc::new(DeviceContext { + let device_ctx = guard.entry((*cm_id.as_ptr()).verbs as usize).or_insert_with(|| { + Arc::new(DeviceContext { // Safe due to the is_null() check above. context: NonNull::new((*cm_id.as_ptr()).verbs).unwrap(), - })); + }) + }); Some(device_ctx.clone()) } @@ -1156,4 +1156,45 @@ mod tests { Err(_) => Ok(()), } } + + #[test] + fn test_get_device_context_caches_correctly() -> Result<(), Box> { + match EventChannel::new() { + Ok(channel) => { + let id = channel.create_id(PortSpace::Tcp)?; + + let _ = id.resolve_addr( + None, + SocketAddr::from((IpAddr::from_str("127.0.0.1")?, 0)), + Duration::new(0, 200000000), + ); + + let event = channel.get_cm_event()?; + assert_eq!(event.event_type(), EventType::AddressResolved); + + let ctx1 = id.get_device_context(); + let ctx2 = id.get_device_context(); + let ctx3 = id.get_device_context(); + + assert!(ctx1.is_some(), "First get_device_context should return Some"); + assert!(ctx2.is_some(), "Second get_device_context should return Some"); + assert!(ctx3.is_some(), "Third get_device_context should return Some"); + + assert!( + Arc::ptr_eq(&ctx1.clone().unwrap(), &ctx2.clone().unwrap()), + "ctx1 and ctx2 should point to the same DeviceContext" + ); + assert!( + Arc::ptr_eq(&ctx2.clone().unwrap(), &ctx3.clone().unwrap()), + "ctx2 and ctx3 should point to the same DeviceContext" + ); + + let ctx = ctx1.unwrap(); + let _pd = ctx.alloc_pd()?; + + Ok(()) + }, + Err(_) => Ok(()), + } + } }