Skip to content

Commit 73ea8e1

Browse files
committed
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 <lukedyue@gmail.com>
1 parent ce99bd9 commit 73ea8e1

1 file changed

Lines changed: 45 additions & 4 deletions

File tree

src/rdmacm/communication_manager.rs

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -847,12 +847,12 @@ impl Identifier {
847847
}
848848

849849
let mut guard = DEVICE_LISTS.lock().unwrap();
850-
let device_ctx = guard
851-
.entry((*cm_id.as_ptr()).verbs as usize)
852-
.or_insert(Arc::new(DeviceContext {
850+
let device_ctx = guard.entry((*cm_id.as_ptr()).verbs as usize).or_insert_with(|| {
851+
Arc::new(DeviceContext {
853852
// Safe due to the is_null() check above.
854853
context: NonNull::new((*cm_id.as_ptr()).verbs).unwrap(),
855-
}));
854+
})
855+
});
856856

857857
Some(device_ctx.clone())
858858
}
@@ -1156,4 +1156,45 @@ mod tests {
11561156
Err(_) => Ok(()),
11571157
}
11581158
}
1159+
1160+
#[test]
1161+
fn test_get_device_context_caches_correctly() -> Result<(), Box<dyn std::error::Error>> {
1162+
match EventChannel::new() {
1163+
Ok(channel) => {
1164+
let id = channel.create_id(PortSpace::Tcp)?;
1165+
1166+
let _ = id.resolve_addr(
1167+
None,
1168+
SocketAddr::from((IpAddr::from_str("127.0.0.1")?, 0)),
1169+
Duration::new(0, 200000000),
1170+
);
1171+
1172+
let event = channel.get_cm_event()?;
1173+
assert_eq!(event.event_type(), EventType::AddressResolved);
1174+
1175+
let ctx1 = id.get_device_context();
1176+
let ctx2 = id.get_device_context();
1177+
let ctx3 = id.get_device_context();
1178+
1179+
assert!(ctx1.is_some(), "First get_device_context should return Some");
1180+
assert!(ctx2.is_some(), "Second get_device_context should return Some");
1181+
assert!(ctx3.is_some(), "Third get_device_context should return Some");
1182+
1183+
assert!(
1184+
Arc::ptr_eq(&ctx1.clone().unwrap(), &ctx2.clone().unwrap()),
1185+
"ctx1 and ctx2 should point to the same DeviceContext"
1186+
);
1187+
assert!(
1188+
Arc::ptr_eq(&ctx2.clone().unwrap(), &ctx3.clone().unwrap()),
1189+
"ctx2 and ctx3 should point to the same DeviceContext"
1190+
);
1191+
1192+
let ctx = ctx1.unwrap();
1193+
let _pd = ctx.alloc_pd()?;
1194+
1195+
Ok(())
1196+
},
1197+
Err(_) => Ok(()),
1198+
}
1199+
}
11591200
}

0 commit comments

Comments
 (0)