From 1aa5ff45d6881b158b0150d79dd8070393f02331 Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 19 Jul 2026 17:12:14 -0700 Subject: [PATCH] [spr] initial version Created using spr 1.3.6-beta.1 --- CHANGELOG.md | 6 +++ crates/iddqd/src/id_hash_map/entry.rs | 3 +- crates/iddqd/src/id_hash_map/imp.rs | 11 ++++++ .../iddqd/tests/integration/pathological.rs | 38 +++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e928f081..a45d6443 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ - MSRV updated to Rust 1.86. +### Fixed + +- The `insert_overwrite` path on `IdHashMap` no longer aborts when an allocation fails, matching the existing guarantee on `BiHashMap` and `TriHashMap`. Instead, it results in a catchable panic. (The map is left unchanged, similar to `BiHashMap` and `TriHashMap`.) + + Note that `BTreeMap::insert_overwrite` will abort on allocation failure, because it calls into `std` which doesn't have an equivalent to `HashMap::try_reserve`. + ## [0.4.5] - 2026-06-17 ### Added diff --git a/crates/iddqd/src/id_hash_map/entry.rs b/crates/iddqd/src/id_hash_map/entry.rs index dcb3ad7a..107fde38 100644 --- a/crates/iddqd/src/id_hash_map/entry.rs +++ b/crates/iddqd/src/id_hash_map/entry.rs @@ -143,7 +143,8 @@ impl<'a, T: IdHashItem, S: Clone + BuildHasher, A: Allocator> // SAFETY: The safety assumption behind `Self::new` guarantees that the // original reference to the map is not used at this point. let map = unsafe { self.map.awaken() }; - map.tables.key_to_item.reserve(1); + map.try_reserve_insert_overwrite_commit() + .expect("reserved space successfully"); let next_index = map.items.assert_can_grow().insert(value); map.tables .key_to_item diff --git a/crates/iddqd/src/id_hash_map/imp.rs b/crates/iddqd/src/id_hash_map/imp.rs index f3484507..926f085e 100644 --- a/crates/iddqd/src/id_hash_map/imp.rs +++ b/crates/iddqd/src/id_hash_map/imp.rs @@ -1466,6 +1466,17 @@ impl IdHashMap { Ok(next_index) } + pub(super) fn try_reserve_insert_overwrite_commit( + &mut self, + ) -> Result<(), crate::errors::TryReserveError> { + self.items.try_reserve(1)?; + self.tables + .key_to_item + .try_reserve(1) + .map_err(crate::errors::TryReserveError::from_hashbrown)?; + Ok(()) + } + pub(super) fn remove_by_index( &mut self, remove_index: ItemIndex, diff --git a/crates/iddqd/tests/integration/pathological.rs b/crates/iddqd/tests/integration/pathological.rs index c06c1066..5d94d9f6 100644 --- a/crates/iddqd/tests/integration/pathological.rs +++ b/crates/iddqd/tests/integration/pathological.rs @@ -1599,4 +1599,42 @@ mod allocator_tests { map.validate(ValidateCompact::Compact) .expect("map remains valid and compact after a failed reservation"); } + + #[test] + fn id_hash_insert_overwrite_atomic_on_alloc_failure() { + let mut map: IdHashMap<_, _, FailingAlloc> = + IdHashMap::with_hasher_in( + foldhash::fast::FixedState::with_seed(0), + FailingAlloc(Global), + ); + for id in 0..4u32 { + map.insert_unique(ForgettableHashItem { id }).unwrap(); + } + map.shrink_to_fit(); + + let before: BTreeSet = map.iter().map(|item| item.id).collect(); + + let result = catch_panic(AssertUnwindSafe(|| { + with_failing_alloc(|| { + map.insert_overwrite(ForgettableHashItem { id: 99 }) + }) + })); + assert!( + result.is_none(), + "insert_overwrite should panic when the reservation fails" + ); + + let after: BTreeSet = map.iter().map(|item| item.id).collect(); + assert_eq!( + after, before, + "map must be unchanged after a failed reserve" + ); + assert_eq!(map.len(), 4, "len is unchanged after a failed reserve"); + assert!( + map.get(&ForgettableHashKey(99)).is_none(), + "the new key must be absent after a failed reserve" + ); + map.validate(ValidateCompact::Compact) + .expect("map remains valid and compact after a failed reservation"); + } }