diff --git a/Cargo.lock b/Cargo.lock index 690de825..49aa08bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -71,7 +71,7 @@ dependencies = [ [[package]] name = "dashmap" -version = "6.1.0" +version = "7.0.0" dependencies = [ "arbitrary", "cfg-if", @@ -93,9 +93,9 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "hashbrown" -version = "0.14.2" +version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f93e7192158dbcda357bdec5fb5788eebf8bbac027f3f33e719d29135ae84156" +checksum = "1e087f84d4f86bf4b218b927129862374b72199ae7d8657835f1e89000eea4fb" [[package]] name = "hermit-abi" diff --git a/Cargo.toml b/Cargo.toml index b29c8da7..a0cdec73 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "dashmap" -version = "6.1.0" +version = "7.0.0" authors = ["Acrimon "] edition = "2018" rust-version = "1.65" @@ -21,7 +21,7 @@ inline = ["hashbrown/inline-more"] [dependencies] lock_api = "0.4.10" parking_lot_core = "0.9.8" -hashbrown = { version = "0.14.0", default-features = false, features = ["raw"] } +hashbrown = { version = "0.15.0", default-features = false } serde = { version = "1.0.188", optional = true, features = ["derive"] } cfg-if = "1.0.0" rayon = { version = "1.7.0", optional = true } diff --git a/src/iter.rs b/src/iter.rs index d069441e..81ca4789 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -1,8 +1,7 @@ use super::mapref::multiple::{RefMulti, RefMutMulti}; -use crate::lock::{RwLockReadGuard, RwLockWriteGuard}; +use crate::lock::{RwLockReadGuardDetached, RwLockWriteGuardDetached}; use crate::t::Map; -use crate::util::SharedValue; -use crate::{DashMap, HashMap}; +use crate::DashMap; use core::hash::{BuildHasher, Hash}; use core::mem; use std::collections::hash_map::RandomState; @@ -38,7 +37,7 @@ impl OwningIter { } } -type GuardOwningIter = hashbrown::raw::RawIntoIter<(K, SharedValue)>; +type GuardOwningIter = hashbrown::hash_table::IntoIter<(K, V)>; impl Iterator for OwningIter { type Item = (K, V); @@ -46,8 +45,8 @@ impl Iterator for OwningIter { fn next(&mut self) -> Option { loop { if let Some(current) = self.current.as_mut() { - if let Some((k, v)) = current.next() { - return Some((k, v.into_inner())); + if let Some(value) = current.next() { + return Some(value); } } @@ -55,7 +54,7 @@ impl Iterator for OwningIter { return None; } - //let guard = unsafe { self.map._yield_read_shard(self.shard_i) }; + // Safety: shard index is in bounds let mut shard_wl = unsafe { self.map._yield_write_shard(self.shard_i) }; let map = mem::take(&mut *shard_wl); @@ -64,7 +63,6 @@ impl Iterator for OwningIter { let iter = map.into_iter(); - //unsafe { ptr::write(&mut self.current, Some((arcee, iter))); } self.current = Some(iter); self.shard_i += 1; @@ -72,30 +70,14 @@ impl Iterator for OwningIter { } } -unsafe impl Send for OwningIter -where - K: Eq + Hash + Send, - V: Send, - S: BuildHasher + Clone + Send, -{ -} - -unsafe impl Sync for OwningIter -where - K: Eq + Hash + Sync, - V: Sync, - S: BuildHasher + Clone + Sync, -{ -} - type GuardIter<'a, K, V> = ( - Arc>>, - hashbrown::raw::RawIter<(K, SharedValue)>, + Arc>, + hashbrown::hash_table::Iter<'a, (K, V)>, ); type GuardIterMut<'a, K, V> = ( - Arc>>, - hashbrown::raw::RawIter<(K, SharedValue)>, + Arc>, + hashbrown::hash_table::IterMut<'a, (K, V)>, ); /// Iterator over a DashMap yielding immutable references. @@ -122,24 +104,6 @@ impl<'i, K: Clone + Hash + Eq, V: Clone, S: Clone + BuildHasher> Clone for Iter< } } -unsafe impl<'a, 'i, K, V, S, M> Send for Iter<'i, K, V, S, M> -where - K: 'a + Eq + Hash + Send, - V: 'a + Send, - S: 'a + BuildHasher + Clone, - M: Map<'a, K, V, S>, -{ -} - -unsafe impl<'a, 'i, K, V, S, M> Sync for Iter<'i, K, V, S, M> -where - K: 'a + Eq + Hash + Sync, - V: 'a + Sync, - S: 'a + BuildHasher + Clone, - M: Map<'a, K, V, S>, -{ -} - impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iter<'a, K, V, S, M> { pub(crate) fn new(map: &'a M) -> Self { Self { @@ -159,12 +123,10 @@ impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iter fn next(&mut self) -> Option { loop { if let Some(current) = self.current.as_mut() { - if let Some(b) = current.1.next() { - return unsafe { - let (k, v) = b.as_ref(); - let guard = current.0.clone(); - Some(RefMulti::new(guard, k, v.get())) - }; + if let Some(data) = current.1.next() { + let guard = current.0.clone(); + // Safety: The guard still protects the data + return unsafe { Some(RefMulti::new(guard, data)) }; } } @@ -172,11 +134,12 @@ impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iter return None; } + // Safety: shard index is in bounds let guard = unsafe { self.map._yield_read_shard(self.shard_i) }; + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockReadGuardDetached::detach_from(guard) }; - let iter = unsafe { guard.iter() }; - - self.current = Some((Arc::new(guard), iter)); + self.current = Some((Arc::new(guard), data.iter())); self.shard_i += 1; } @@ -202,24 +165,6 @@ pub struct IterMut<'a, K, V, S = RandomState, M = DashMap> { marker: PhantomData, } -unsafe impl<'a, 'i, K, V, S, M> Send for IterMut<'i, K, V, S, M> -where - K: 'a + Eq + Hash + Send, - V: 'a + Send, - S: 'a + BuildHasher + Clone, - M: Map<'a, K, V, S>, -{ -} - -unsafe impl<'a, 'i, K, V, S, M> Sync for IterMut<'i, K, V, S, M> -where - K: 'a + Eq + Hash + Sync, - V: 'a + Sync, - S: 'a + BuildHasher + Clone, - M: Map<'a, K, V, S>, -{ -} - impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> IterMut<'a, K, V, S, M> { @@ -241,12 +186,10 @@ impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iter fn next(&mut self) -> Option { loop { if let Some(current) = self.current.as_mut() { - if let Some(b) = current.1.next() { - return unsafe { - let (k, v) = b.as_mut(); - let guard = current.0.clone(); - Some(RefMutMulti::new(guard, k, v.get_mut())) - }; + if let Some(data) = current.1.next() { + let guard = current.0.clone(); + // Safety: The guard still protects the data + return unsafe { Some(RefMutMulti::new(guard, data)) }; } } @@ -254,11 +197,12 @@ impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iter return None; } + // Safety: shard index is in bounds let guard = unsafe { self.map._yield_write_shard(self.shard_i) }; + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockWriteGuardDetached::detach_from(guard) }; - let iter = unsafe { guard.iter() }; - - self.current = Some((Arc::new(guard), iter)); + self.current = Some((Arc::new(guard), data.iter_mut())); self.shard_i += 1; } @@ -280,7 +224,7 @@ mod tests { let mut c = 0; for shard in map.shards() { - c += unsafe { shard.write().iter().count() }; + c += shard.write().iter().count(); } assert_eq!(c, 1); diff --git a/src/iter_set.rs b/src/iter_set.rs index 98b930c3..9ae2b9b5 100644 --- a/src/iter_set.rs +++ b/src/iter_set.rs @@ -20,40 +20,10 @@ impl Iterator for OwningIter { } } -unsafe impl Send for OwningIter -where - K: Eq + Hash + Send, - S: BuildHasher + Clone + Send, -{ -} - -unsafe impl Sync for OwningIter -where - K: Eq + Hash + Sync, - S: BuildHasher + Clone + Sync, -{ -} - pub struct Iter<'a, K, S, M> { inner: crate::iter::Iter<'a, K, (), S, M>, } -unsafe impl<'a, 'i, K, S, M> Send for Iter<'i, K, S, M> -where - K: 'a + Eq + Hash + Send, - S: 'a + BuildHasher + Clone, - M: Map<'a, K, (), S>, -{ -} - -unsafe impl<'a, 'i, K, S, M> Sync for Iter<'i, K, S, M> -where - K: 'a + Eq + Hash + Sync, - S: 'a + BuildHasher + Clone, - M: Map<'a, K, (), S>, -{ -} - impl<'a, K: Eq + Hash, S: 'a + BuildHasher + Clone, M: Map<'a, K, (), S>> Iter<'a, K, S, M> { pub(crate) fn new(inner: crate::iter::Iter<'a, K, (), S, M>) -> Self { Self { inner } diff --git a/src/lib.rs b/src/lib.rs index c0abb5e6..40c75389 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![allow(clippy::type_complexity)] +#![warn(unsafe_op_in_unsafe_fn, unused_unsafe, clippy::missing_safety_doc)] #[cfg(feature = "arbitrary")] mod arbitrary; @@ -22,11 +22,10 @@ pub mod rayon { pub mod set; } -#[cfg(not(feature = "raw-api"))] -use crate::lock::{RwLock, RwLockReadGuard, RwLockWriteGuard}; - #[cfg(feature = "raw-api")] pub use crate::lock::{RawRwLock, RwLock, RwLockReadGuard, RwLockWriteGuard}; +#[cfg(not(feature = "raw-api"))] +use crate::lock::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use cfg_if::cfg_if; use core::borrow::Borrow; @@ -36,6 +35,7 @@ use core::iter::FromIterator; use core::ops::{BitAnd, BitOr, Shl, Shr, Sub}; use crossbeam_utils::CachePadded; use iter::{Iter, IterMut, OwningIter}; +use lock::{RwLockReadGuardDetached, RwLockWriteGuardDetached}; pub use mapref::entry::{Entry, OccupiedEntry, VacantEntry}; use mapref::multiple::RefMulti; use mapref::one::{Ref, RefMut}; @@ -46,15 +46,8 @@ use std::collections::hash_map::RandomState; pub use t::Map; use try_result::TryResult; -cfg_if! { - if #[cfg(feature = "raw-api")] { - pub use util::SharedValue; - } else { - use util::SharedValue; - } -} - -pub(crate) type HashMap = hashbrown::raw::RawTable<(K, SharedValue)>; +pub(crate) type HashMap = hashbrown::HashTable<(K, V)>; +pub(crate) type Shard = CachePadded>>; // Temporary reimplementation of [`std::collections::TryReserveError`] // util [`std::collections::TryReserveError`] stabilises. @@ -88,7 +81,7 @@ fn ncb(shard_amount: usize) -> usize { /// This means that it is safe to ignore it across multiple threads. pub struct DashMap { shift: usize, - shards: Box<[CachePadded>>]>, + shards: Box<[Shard]>, hasher: S, } @@ -322,7 +315,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: BuildHasher + Clone> DashMap { /// let map = DashMap::<(), ()>::new(); /// println!("Amount of shards: {}", map.shards().len()); /// ``` - pub fn shards(&self) -> &[CachePadded>>] { + pub fn shards(&self) -> &[Shard] { &self.shards } @@ -348,10 +341,10 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: BuildHasher + Clone> DashMap { /// }; /// let data = (42, SharedValue::new("forty two")); /// let hash = hasher(&data); - /// map.shards_mut()[shard_ind].get_mut().insert(hash, data, hasher); + /// map.shards_mut()[shard_ind].get_mut().insert_unique(hash, data, hasher); /// assert_eq!(*map.get(&42).unwrap(), "forty two"); /// ``` - pub fn shards_mut(&mut self) -> &mut [CachePadded>>] { + pub fn shards_mut(&mut self) -> &mut [Shard] { &mut self.shards } @@ -361,22 +354,22 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: BuildHasher + Clone> DashMap { /// Requires the `raw-api` feature to be enabled. /// /// See [`DashMap::shards()`] and [`DashMap::shards_mut()`] for more information. - pub fn into_shards(self) -> Box<[CachePadded>>]> { + pub fn into_shards(self) -> Box<[Shard]> { self.shards } } else { #[allow(dead_code)] - pub(crate) fn shards(&self) -> &[CachePadded>>] { + pub(crate) fn shards(&self) -> &[Shard] { &self.shards } #[allow(dead_code)] - pub(crate) fn shards_mut(&mut self) -> &mut [CachePadded>>] { + pub(crate) fn shards_mut(&mut self) -> &mut [Shard] { &mut self.shards } #[allow(dead_code)] - pub(crate) fn into_shards(self) -> Box<[CachePadded>>]> { + pub(crate) fn into_shards(self) -> Box<[Shard]> { self.shards } } @@ -910,40 +903,48 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> self.shards.len() } + /// Safety: + /// * `i` must be in bounds + /// * `i` must be read-only - no write locks allowed unsafe fn _get_read_shard(&'a self, i: usize) -> &'a HashMap { debug_assert!(i < self.shards.len()); - &*self.shards.get_unchecked(i).data_ptr() + let shard = unsafe { self.shards.get_unchecked(i) }; + unsafe { &*shard.data_ptr() } } + /// Safety: `i` must be in bounds unsafe fn _yield_read_shard(&'a self, i: usize) -> RwLockReadGuard<'a, HashMap> { debug_assert!(i < self.shards.len()); - self.shards.get_unchecked(i).read() + unsafe { self.shards.get_unchecked(i) }.read() } + /// Safety: `i` must be in bounds unsafe fn _yield_write_shard(&'a self, i: usize) -> RwLockWriteGuard<'a, HashMap> { debug_assert!(i < self.shards.len()); - self.shards.get_unchecked(i).write() + unsafe { self.shards.get_unchecked(i) }.write() } + /// Safety: `i` must be in bounds unsafe fn _try_yield_read_shard( &'a self, i: usize, ) -> Option>> { debug_assert!(i < self.shards.len()); - self.shards.get_unchecked(i).try_read() + unsafe { self.shards.get_unchecked(i) }.try_read() } + /// Safety: `i` must be in bounds unsafe fn _try_yield_write_shard( &'a self, i: usize, ) -> Option>> { debug_assert!(i < self.shards.len()); - self.shards.get_unchecked(i).try_write() + unsafe { self.shards.get_unchecked(i) }.try_write() } fn _insert(&self, key: K, value: V) -> Option { @@ -965,13 +966,15 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); + // Safety: shard index is in bounds let mut shard = unsafe { self._yield_write_shard(idx) }; - if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) { - let ((k, v), _) = unsafe { shard.remove(bucket) }; - Some((k, v.into_inner())) - } else { - None + match shard.find_entry(hash, |(k, _v)| key == k.borrow()) { + Ok(entry) => { + let (val, _) = entry.remove(); + Some(val) + } + Err(_) => None, } } @@ -984,18 +987,20 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); + // Safety: shard index is in bounds let mut shard = unsafe { self._yield_write_shard(idx) }; - if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) { - let (k, v) = unsafe { bucket.as_ref() }; - if f(k, v.get()) { - let ((k, v), _) = unsafe { shard.remove(bucket) }; - Some((k, v.into_inner())) - } else { - None + match shard.find_entry(hash, |(k, _v)| key == k.borrow()) { + Ok(entry) => { + let (k, v) = entry.get(); + if f(k, v) { + let (val, _) = entry.remove(); + Some(val) + } else { + None + } } - } else { - None + Err(_) => None, } } @@ -1008,18 +1013,20 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); + // Safety: shard index is in bounds let mut shard = unsafe { self._yield_write_shard(idx) }; - if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) { - let (k, v) = unsafe { bucket.as_mut() }; - if f(k, v.get_mut()) { - let ((k, v), _) = unsafe { shard.remove(bucket) }; - Some((k, v.into_inner())) - } else { - None + match shard.find_entry(hash, |(k, _v)| key == k.borrow()) { + Ok(mut entry) => { + let (k, v) = entry.get_mut(); + if f(k, v) { + let (val, _) = entry.remove(); + Some(val) + } else { + None + } } - } else { - None + Err(_) => None, } } @@ -1040,16 +1047,14 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); + // Safety: shard index is in bounds let shard = unsafe { self._yield_read_shard(idx) }; + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockReadGuardDetached::detach_from(shard) }; - if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) { - unsafe { - let (k, v) = bucket.as_ref(); - Some(Ref::new(shard, k, v.as_ptr())) - } - } else { - None - } + data.find(hash, |(k, _v)| key == k.borrow()) + // Safety: The guard is still protecting the data. + .map(|data| unsafe { Ref::new(guard, data) }) } fn _get_mut(&'a self, key: &Q) -> Option> @@ -1061,16 +1066,13 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); + // Safety: shard index is in bounds let shard = unsafe { self._yield_write_shard(idx) }; + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockWriteGuardDetached::detach_from(shard) }; - if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) { - unsafe { - let (k, v) = bucket.as_ref(); - Some(RefMut::new(shard, k, v.as_ptr())) - } - } else { - None - } + data.find_mut(hash, |(k, _v)| key == k.borrow()) + .map(|data| unsafe { RefMut::new(guard, data) }) } fn _try_get(&'a self, key: &Q) -> TryResult> @@ -1082,18 +1084,19 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); + // Safety: shard index is in bounds let shard = match unsafe { self._try_yield_read_shard(idx) } { Some(shard) => shard, None => return TryResult::Locked, }; - if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) { - unsafe { - let (k, v) = bucket.as_ref(); - TryResult::Present(Ref::new(shard, k, v.as_ptr())) - } - } else { - TryResult::Absent + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockReadGuardDetached::detach_from(shard) }; + + match data.find(hash, |(k, _v)| key == k.borrow()) { + // Safety: The guard is still protecting the data. + Some(data) => TryResult::Present(unsafe { Ref::new(guard, data) }), + None => TryResult::Absent, } } @@ -1106,18 +1109,19 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); + // Safety: shard index is in bounds let shard = match unsafe { self._try_yield_write_shard(idx) } { Some(shard) => shard, None => return TryResult::Locked, }; - if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) { - unsafe { - let (k, v) = bucket.as_ref(); - TryResult::Present(RefMut::new(shard, k, v.as_ptr())) - } - } else { - TryResult::Absent + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockWriteGuardDetached::detach_from(shard) }; + + match data.find_mut(hash, |(k, _v)| key == k.borrow()) { + // Safety: The guard is still protecting the data. + Some(data) => TryResult::Present(unsafe { RefMut::new(guard, data) }), + None => TryResult::Absent, } } @@ -1135,16 +1139,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> fn _retain(&self, mut f: impl FnMut(&K, &mut V) -> bool) { self.shards.iter().for_each(|s| { - unsafe { - let mut shard = s.write(); - // Here we only use `iter` as a temporary, preventing use-after-free - for bucket in shard.iter() { - let (k, v) = bucket.as_mut(); - if !f(&*k, v.get_mut()) { - shard.erase(bucket); - } - } - } + s.write().retain(|(k, v)| f(&*k, v)); }); } @@ -1187,9 +1182,12 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); - let mut shard = unsafe { self._yield_write_shard(idx) }; + // Safety: shard index is in bounds + let shard = unsafe { self._yield_write_shard(idx) }; + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockWriteGuardDetached::detach_from(shard) }; - match shard.find_or_find_insert_slot( + match data.entry( hash, |(k, _v)| k == &key, |(k, _v)| { @@ -1198,8 +1196,14 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> hasher.finish() }, ) { - Ok(elem) => Entry::Occupied(unsafe { OccupiedEntry::new(shard, key, elem) }), - Err(slot) => Entry::Vacant(unsafe { VacantEntry::new(shard, key, hash, slot) }), + hashbrown::hash_table::Entry::Occupied(occupied_entry) => { + // Safety: The guard is still protecting the data. + Entry::Occupied(unsafe { OccupiedEntry::new(guard, occupied_entry, key) }) + } + hashbrown::hash_table::Entry::Vacant(vacant_entry) => { + // Safety: The guard is still protecting the data. + Entry::Vacant(unsafe { VacantEntry::new(guard, vacant_entry, key) }) + } } } @@ -1208,12 +1212,16 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> let idx = self.determine_shard(hash as usize); - let mut shard = match unsafe { self._try_yield_write_shard(idx) } { + // Safety: shard index is in bounds + let shard = match unsafe { self._try_yield_write_shard(idx) } { Some(shard) => shard, None => return None, }; - match shard.find_or_find_insert_slot( + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockWriteGuardDetached::detach_from(shard) }; + + match data.entry( hash, |(k, _v)| k == &key, |(k, _v)| { @@ -1222,11 +1230,15 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S> hasher.finish() }, ) { - Ok(elem) => Some(Entry::Occupied(unsafe { - OccupiedEntry::new(shard, key, elem) - })), - Err(slot) => Some(Entry::Vacant(unsafe { - VacantEntry::new(shard, key, hash, slot) + hashbrown::hash_table::Entry::Occupied(occupied_entry) => { + // Safety: The guard is still protecting the data. + Some(Entry::Occupied(unsafe { + OccupiedEntry::new(guard, occupied_entry, key) + })) + } + // Safety: The guard is still protecting the data. + hashbrown::hash_table::Entry::Vacant(vacant_entry) => Some(Entry::Vacant(unsafe { + VacantEntry::new(guard, vacant_entry, key) })), } } @@ -1359,15 +1371,9 @@ where .iter() .map(|shard_lock| { let shard = shard_lock.read(); - let hashtable_size = shard.allocation_info().1.size(); - - // Safety: The iterator is dropped before the HashTable - let iter = unsafe { shard.iter() }; - let entry_size_iter = iter.map(|bucket| { - // Safety: The iterator returns buckets with valid pointers to entries - let (key, value) = unsafe { bucket.as_ref() }; - key.extra_size() + value.get().extra_size() - }); + let hashtable_size = shard.allocation_size(); + + let entry_size_iter = shard.iter().map(typesize::TypeSize::extra_size); core::mem::size_of::>>>() + hashtable_size diff --git a/src/lock.rs b/src/lock.rs index f2c98c76..2b094fd1 100644 --- a/src/lock.rs +++ b/src/lock.rs @@ -4,6 +4,8 @@ use parking_lot_core::{ParkToken, SpinWait, UnparkToken}; pub type RwLock = lock_api::RwLock; pub type RwLockReadGuard<'a, T> = lock_api::RwLockReadGuard<'a, RawRwLock, T>; pub type RwLockWriteGuard<'a, T> = lock_api::RwLockWriteGuard<'a, RawRwLock, T>; +pub(crate) type RwLockReadGuardDetached<'a> = crate::util::RwLockReadGuardDetached<'a, RawRwLock>; +pub(crate) type RwLockWriteGuardDetached<'a> = crate::util::RwLockWriteGuardDetached<'a, RawRwLock>; const READERS_PARKED: usize = 0b0001; const WRITERS_PARKED: usize = 0b0010; @@ -20,7 +22,7 @@ unsafe impl lock_api::RawRwLock for RawRwLock { state: AtomicUsize::new(0), }; - type GuardMarker = lock_api::GuardNoSend; + type GuardMarker = lock_api::GuardSend; #[inline] fn try_lock_exclusive(&self) -> bool { @@ -80,7 +82,9 @@ unsafe impl lock_api::RawRwLockDowngrade for RawRwLock { .state .fetch_and(ONE_READER | WRITERS_PARKED, Ordering::Release); if state & READERS_PARKED != 0 { - parking_lot_core::unpark_all((self as *const _ as usize) + 1, UnparkToken(0)); + unsafe { + parking_lot_core::unpark_all((self as *const _ as usize) + 1, UnparkToken(0)); + } } } } diff --git a/src/mapref/entry.rs b/src/mapref/entry.rs index f4cc6819..4f31fd32 100644 --- a/src/mapref/entry.rs +++ b/src/mapref/entry.rs @@ -1,9 +1,8 @@ use super::one::RefMut; -use crate::lock::RwLockWriteGuard; -use crate::util::SharedValue; -use crate::HashMap; +use crate::lock::RwLockWriteGuardDetached; use core::hash::Hash; use core::mem; +use hashbrown::hash_table; pub enum Entry<'a, K, V> { Occupied(OccupiedEntry<'a, K, V>), @@ -112,58 +111,37 @@ impl<'a, K: Eq + Hash, V> Entry<'a, K, V> { } pub struct VacantEntry<'a, K, V> { - shard: RwLockWriteGuard<'a, HashMap>, + guard: RwLockWriteGuardDetached<'a>, + entry: hash_table::VacantEntry<'a, (K, V)>, key: K, - hash: u64, - slot: hashbrown::raw::InsertSlot, } -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Send for VacantEntry<'a, K, V> {} -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Sync for VacantEntry<'a, K, V> {} - impl<'a, K: Eq + Hash, V> VacantEntry<'a, K, V> { + /// # Safety + /// The guard should be protecting the provided entry. pub(crate) unsafe fn new( - shard: RwLockWriteGuard<'a, HashMap>, + guard: RwLockWriteGuardDetached<'a>, + entry: hash_table::VacantEntry<'a, (K, V)>, key: K, - hash: u64, - slot: hashbrown::raw::InsertSlot, ) -> Self { - Self { - shard, - key, - hash, - slot, - } + Self { guard, entry, key } } - pub fn insert(mut self, value: V) -> RefMut<'a, K, V> { - unsafe { - let occupied = self.shard.insert_in_slot( - self.hash, - self.slot, - (self.key, SharedValue::new(value)), - ); - - let (k, v) = occupied.as_ref(); + pub fn insert(self, value: V) -> RefMut<'a, K, V> { + let occupied = self.entry.insert((self.key, value)); - RefMut::new(self.shard, k, v.as_ptr()) - } + // Safety: The guard is still protecting the occupied entry data. + unsafe { RefMut::new(self.guard, occupied.into_mut()) } } /// Sets the value of the entry with the VacantEntry’s key, and returns an OccupiedEntry. - pub fn insert_entry(mut self, value: V) -> OccupiedEntry<'a, K, V> + pub fn insert_entry(self, value: V) -> OccupiedEntry<'a, K, V> where K: Clone, { - unsafe { - let bucket = self.shard.insert_in_slot( - self.hash, - self.slot, - (self.key.clone(), SharedValue::new(value)), - ); - - OccupiedEntry::new(self.shard, self.key, bucket) - } + let entry = self.entry.insert((self.key.clone(), value)); + // Safety: The guard is still protecting the occupied entry. + unsafe { OccupiedEntry::new(self.guard, entry, self.key) } } pub fn into_key(self) -> K { @@ -176,29 +154,30 @@ impl<'a, K: Eq + Hash, V> VacantEntry<'a, K, V> { } pub struct OccupiedEntry<'a, K, V> { - shard: RwLockWriteGuard<'a, HashMap>, - bucket: hashbrown::raw::Bucket<(K, SharedValue)>, + guard: RwLockWriteGuardDetached<'a>, + entry: hash_table::OccupiedEntry<'a, (K, V)>, key: K, } -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Send for OccupiedEntry<'a, K, V> {} -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Sync for OccupiedEntry<'a, K, V> {} - impl<'a, K: Eq + Hash, V> OccupiedEntry<'a, K, V> { + /// # Safety + /// The guard should be protecting the provided entry. pub(crate) unsafe fn new( - shard: RwLockWriteGuard<'a, HashMap>, + guard: RwLockWriteGuardDetached<'a>, + entry: hash_table::OccupiedEntry<'a, (K, V)>, key: K, - bucket: hashbrown::raw::Bucket<(K, SharedValue)>, ) -> Self { - Self { shard, bucket, key } + Self { guard, entry, key } } pub fn get(&self) -> &V { - unsafe { self.bucket.as_ref().1.get() } + let (_k, v) = self.entry.get(); + v } pub fn get_mut(&mut self) -> &mut V { - unsafe { self.bucket.as_mut().1.get_mut() } + let (_k, v) = self.entry.get_mut(); + v } pub fn insert(&mut self, value: V) -> V { @@ -206,10 +185,8 @@ impl<'a, K: Eq + Hash, V> OccupiedEntry<'a, K, V> { } pub fn into_ref(self) -> RefMut<'a, K, V> { - unsafe { - let (k, v) = self.bucket.as_ref(); - RefMut::new(self.shard, k, v.as_ptr()) - } + // Safety: The guard is still protecting the occupied entry data. + unsafe { RefMut::new(self.guard, self.entry.into_mut()) } } pub fn into_key(self) -> K { @@ -217,25 +194,22 @@ impl<'a, K: Eq + Hash, V> OccupiedEntry<'a, K, V> { } pub fn key(&self) -> &K { - unsafe { &self.bucket.as_ref().0 } + &self.entry.get().0 } - pub fn remove(mut self) -> V { - let ((_k, v), _) = unsafe { self.shard.remove(self.bucket) }; - v.into_inner() + pub fn remove(self) -> V { + let ((_k, v), _) = self.entry.remove(); + v } - pub fn remove_entry(mut self) -> (K, V) { - let ((k, v), _) = unsafe { self.shard.remove(self.bucket) }; - (k, v.into_inner()) + pub fn remove_entry(self) -> (K, V) { + let (entry, _) = self.entry.remove(); + entry } pub fn replace_entry(self, value: V) -> (K, V) { - let (k, v) = mem::replace( - unsafe { self.bucket.as_mut() }, - (self.key, SharedValue::new(value)), - ); - (k, v.into_inner()) + let entry = mem::replace(self.entry.into_mut(), (self.key, value)); + entry } } diff --git a/src/mapref/multiple.rs b/src/mapref/multiple.rs index 1a721b0d..59c08099 100644 --- a/src/mapref/multiple.rs +++ b/src/mapref/multiple.rs @@ -1,28 +1,18 @@ -use crate::lock::{RwLockReadGuard, RwLockWriteGuard}; -use crate::HashMap; +use crate::lock::{RwLockReadGuardDetached, RwLockWriteGuardDetached}; use core::hash::Hash; use core::ops::{Deref, DerefMut}; use std::sync::Arc; pub struct RefMulti<'a, K, V> { - _guard: Arc>>, - k: *const K, - v: *const V, + _guard: Arc>, + data: &'a (K, V), } -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Send for RefMulti<'a, K, V> {} -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Sync for RefMulti<'a, K, V> {} - impl<'a, K: Eq + Hash, V> RefMulti<'a, K, V> { - pub(crate) unsafe fn new( - guard: Arc>>, - k: *const K, - v: *const V, - ) -> Self { + pub(crate) unsafe fn new(guard: Arc>, data: &'a (K, V)) -> Self { Self { _guard: guard, - k, - v, + data, } } @@ -35,7 +25,7 @@ impl<'a, K: Eq + Hash, V> RefMulti<'a, K, V> { } pub fn pair(&self) -> (&K, &V) { - unsafe { (&*self.k, &*self.v) } + (&self.data.0, &self.data.1) } } @@ -48,24 +38,18 @@ impl<'a, K: Eq + Hash, V> Deref for RefMulti<'a, K, V> { } pub struct RefMutMulti<'a, K, V> { - _guard: Arc>>, - k: *const K, - v: *mut V, + _guard: Arc>, + data: &'a mut (K, V), } -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Send for RefMutMulti<'a, K, V> {} -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Sync for RefMutMulti<'a, K, V> {} - impl<'a, K: Eq + Hash, V> RefMutMulti<'a, K, V> { pub(crate) unsafe fn new( - guard: Arc>>, - k: *const K, - v: *mut V, + guard: Arc>, + data: &'a mut (K, V), ) -> Self { Self { _guard: guard, - k, - v, + data, } } @@ -82,11 +66,11 @@ impl<'a, K: Eq + Hash, V> RefMutMulti<'a, K, V> { } pub fn pair(&self) -> (&K, &V) { - unsafe { (&*self.k, &*self.v) } + (&self.data.0, &self.data.1) } pub fn pair_mut(&mut self) -> (&K, &mut V) { - unsafe { (&*self.k, &mut *self.v) } + (&self.data.0, &mut self.data.1) } } diff --git a/src/mapref/one.rs b/src/mapref/one.rs index 01393632..3d007ff8 100644 --- a/src/mapref/one.rs +++ b/src/mapref/one.rs @@ -1,62 +1,52 @@ -use crate::lock::{RwLockReadGuard, RwLockWriteGuard}; -use crate::HashMap; +use crate::lock::{RwLockReadGuardDetached, RwLockWriteGuardDetached}; use core::hash::Hash; use core::ops::{Deref, DerefMut}; use std::fmt::{Debug, Formatter}; +use std::ptr::{addr_of, addr_of_mut}; pub struct Ref<'a, K, V> { - _guard: RwLockReadGuard<'a, HashMap>, - k: *const K, - v: *const V, + guard: RwLockReadGuardDetached<'a>, + data: &'a (K, V), } -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Send for Ref<'a, K, V> {} -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Sync for Ref<'a, K, V> {} - impl<'a, K: Eq + Hash, V> Ref<'a, K, V> { - pub(crate) unsafe fn new( - guard: RwLockReadGuard<'a, HashMap>, - k: *const K, - v: *const V, - ) -> Self { - Self { - _guard: guard, - k, - v, - } + /// # Safety + /// The guard should be protecting the provided data. + pub(crate) unsafe fn new(guard: RwLockReadGuardDetached<'a>, data: &'a (K, V)) -> Self { + Self { guard, data } } pub fn key(&self) -> &K { - self.pair().0 + &self.data.0 } pub fn value(&self) -> &V { - self.pair().1 + &self.data.1 } pub fn pair(&self) -> (&K, &V) { - unsafe { (&*self.k, &*self.v) } + (&self.data.0, &self.data.1) } - pub fn map(self, f: F) -> MappedRef<'a, K, V, T> + pub fn map(self, f: F) -> MappedRef<'a, K, T> where F: FnOnce(&V) -> &T, { MappedRef { - _guard: self._guard, - k: self.k, - v: f(unsafe { &*self.v }), + _guard: self.guard, + k: &self.data.0, + v: f(&self.data.1), } } - pub fn try_map(self, f: F) -> Result, Self> + pub fn try_map(self, f: F) -> Result, Self> where F: FnOnce(&V) -> Option<&T>, { - if let Some(v) = f(unsafe { &*self.v }) { + if let Some(v) = f(&self.data.1) { Ok(MappedRef { - _guard: self._guard, - k: self.k, + _guard: self.guard, + k: &self.data.0, v, }) } else { @@ -68,8 +58,8 @@ impl<'a, K: Eq + Hash, V> Ref<'a, K, V> { impl<'a, K: Eq + Hash + Debug, V: Debug> Debug for Ref<'a, K, V> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("Ref") - .field("k", &self.k) - .field("v", &self.v) + .field("k", &self.data.0) + .field("v", &self.data.1) .finish() } } @@ -83,21 +73,15 @@ impl<'a, K: Eq + Hash, V> Deref for Ref<'a, K, V> { } pub struct RefMut<'a, K, V> { - guard: RwLockWriteGuard<'a, HashMap>, - k: *const K, - v: *mut V, + guard: RwLockWriteGuardDetached<'a>, + data: &'a mut (K, V), } -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Send for RefMut<'a, K, V> {} -unsafe impl<'a, K: Eq + Hash + Sync, V: Sync> Sync for RefMut<'a, K, V> {} - impl<'a, K: Eq + Hash, V> RefMut<'a, K, V> { - pub(crate) unsafe fn new( - guard: RwLockWriteGuard<'a, HashMap>, - k: *const K, - v: *mut V, - ) -> Self { - Self { guard, k, v } + /// # Safety + /// The guard should be protecting the provided data. + pub(crate) unsafe fn new(guard: RwLockWriteGuardDetached<'a>, data: &'a mut (K, V)) -> Self { + Self { guard, data } } pub fn key(&self) -> &K { @@ -113,51 +97,56 @@ impl<'a, K: Eq + Hash, V> RefMut<'a, K, V> { } pub fn pair(&self) -> (&K, &V) { - unsafe { (&*self.k, &*self.v) } + (&self.data.0, &self.data.1) } pub fn pair_mut(&mut self) -> (&K, &mut V) { - unsafe { (&*self.k, &mut *self.v) } + (&self.data.0, &mut self.data.1) } pub fn downgrade(self) -> Ref<'a, K, V> { - unsafe { Ref::new(RwLockWriteGuard::downgrade(self.guard), self.k, self.v) } + unsafe { Ref::new(self.guard.downgrade(), &*addr_of!(*self.data)) } } - pub fn map(self, f: F) -> MappedRefMut<'a, K, V, T> + pub fn map(self, f: F) -> MappedRefMut<'a, K, T> where F: FnOnce(&mut V) -> &mut T, { MappedRefMut { _guard: self.guard, - k: self.k, - v: f(unsafe { &mut *self.v }), + k: &self.data.0, + v: f(&mut self.data.1), } } - pub fn try_map(self, f: F) -> Result, Self> + pub fn try_map(self, f: F) -> Result, Self> where - F: FnOnce(&mut V) -> Option<&mut T>, + F: for<'v> FnOnce(&'v mut V) -> Option<&'v mut T>, { - let v = match f(unsafe { &mut *(self.v as *mut _) }) { - Some(v) => v, - None => return Err(self), - }; - let guard = self.guard; - let k = self.k; - Ok(MappedRefMut { - _guard: guard, - k, - v, - }) + // temporarily extend the lifetime to get around issues of returning `Err(self)` + // while "it's still borrowed" + // + // SAFETY: we pass it to `f`, which is not allowed to assume the lifetime lives + // for a long time. We shrink the lifetime of the result value again. + let v = unsafe { &mut *addr_of_mut!(self.data.1) }; + + if let Some(v) = f(v) { + Ok(MappedRefMut { + _guard: self.guard, + k: &self.data.0, + v, + }) + } else { + Err(self) + } } } impl<'a, K: Eq + Hash + Debug, V: Debug> Debug for RefMut<'a, K, V> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("RefMut") - .field("k", &self.k) - .field("v", &self.v) + .field("k", &self.data.0) + .field("v", &self.data.1) .finish() } } @@ -176,13 +165,13 @@ impl<'a, K: Eq + Hash, V> DerefMut for RefMut<'a, K, V> { } } -pub struct MappedRef<'a, K, V, T> { - _guard: RwLockReadGuard<'a, HashMap>, - k: *const K, - v: *const T, +pub struct MappedRef<'a, K, T> { + _guard: RwLockReadGuardDetached<'a>, + k: &'a K, + v: &'a T, } -impl<'a, K: Eq + Hash, V, T> MappedRef<'a, K, V, T> { +impl<'a, K: Eq + Hash, T> MappedRef<'a, K, T> { pub fn key(&self) -> &K { self.pair().0 } @@ -192,25 +181,25 @@ impl<'a, K: Eq + Hash, V, T> MappedRef<'a, K, V, T> { } pub fn pair(&self) -> (&K, &T) { - unsafe { (&*self.k, &*self.v) } + (self.k, self.v) } - pub fn map(self, f: F) -> MappedRef<'a, K, V, T2> + pub fn map(self, f: F) -> MappedRef<'a, K, T2> where F: FnOnce(&T) -> &T2, { MappedRef { _guard: self._guard, k: self.k, - v: f(unsafe { &*self.v }), + v: f(self.v), } } - pub fn try_map(self, f: F) -> Result, Self> + pub fn try_map(self, f: F) -> Result, Self> where F: FnOnce(&T) -> Option<&T2>, { - let v = match f(unsafe { &*self.v }) { + let v = match f(self.v) { Some(v) => v, None => return Err(self), }; @@ -223,7 +212,7 @@ impl<'a, K: Eq + Hash, V, T> MappedRef<'a, K, V, T> { } } -impl<'a, K: Eq + Hash + Debug, V, T: Debug> Debug for MappedRef<'a, K, V, T> { +impl<'a, K: Eq + Hash + Debug, T: Debug> Debug for MappedRef<'a, K, T> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("MappedRef") .field("k", &self.k) @@ -232,7 +221,7 @@ impl<'a, K: Eq + Hash + Debug, V, T: Debug> Debug for MappedRef<'a, K, V, T> { } } -impl<'a, K: Eq + Hash, V, T> Deref for MappedRef<'a, K, V, T> { +impl<'a, K: Eq + Hash, T> Deref for MappedRef<'a, K, T> { type Target = T; fn deref(&self) -> &T { @@ -240,27 +229,25 @@ impl<'a, K: Eq + Hash, V, T> Deref for MappedRef<'a, K, V, T> { } } -impl<'a, K: Eq + Hash, V, T: std::fmt::Display> std::fmt::Display for MappedRef<'a, K, V, T> { +impl<'a, K: Eq + Hash, T: std::fmt::Display> std::fmt::Display for MappedRef<'a, K, T> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { std::fmt::Display::fmt(self.value(), f) } } -impl<'a, K: Eq + Hash, V, T: AsRef, TDeref: ?Sized> AsRef - for MappedRef<'a, K, V, T> -{ +impl<'a, K: Eq + Hash, T: AsRef, TDeref: ?Sized> AsRef for MappedRef<'a, K, T> { fn as_ref(&self) -> &TDeref { self.value().as_ref() } } -pub struct MappedRefMut<'a, K, V, T> { - _guard: RwLockWriteGuard<'a, HashMap>, - k: *const K, - v: *mut T, +pub struct MappedRefMut<'a, K, T> { + _guard: RwLockWriteGuardDetached<'a>, + k: &'a K, + v: &'a mut T, } -impl<'a, K: Eq + Hash, V, T> MappedRefMut<'a, K, V, T> { +impl<'a, K: Eq + Hash, T> MappedRefMut<'a, K, T> { pub fn key(&self) -> &K { self.pair().0 } @@ -274,25 +261,25 @@ impl<'a, K: Eq + Hash, V, T> MappedRefMut<'a, K, V, T> { } pub fn pair(&self) -> (&K, &T) { - unsafe { (&*self.k, &*self.v) } + (self.k, &*self.v) } pub fn pair_mut(&mut self) -> (&K, &mut T) { - unsafe { (&*self.k, &mut *self.v) } + (self.k, self.v) } - pub fn map(self, f: F) -> MappedRefMut<'a, K, V, T2> + pub fn map(self, f: F) -> MappedRefMut<'a, K, T2> where F: FnOnce(&mut T) -> &mut T2, { MappedRefMut { _guard: self._guard, k: self.k, - v: f(unsafe { &mut *self.v }), + v: f(self.v), } } - pub fn try_map(self, f: F) -> Result, Self> + pub fn try_map(self, f: F) -> Result, Self> where F: FnOnce(&mut T) -> Option<&mut T2>, { @@ -310,7 +297,7 @@ impl<'a, K: Eq + Hash, V, T> MappedRefMut<'a, K, V, T> { } } -impl<'a, K: Eq + Hash + Debug, V, T: Debug> Debug for MappedRefMut<'a, K, V, T> { +impl<'a, K: Eq + Hash + Debug, T: Debug> Debug for MappedRefMut<'a, K, T> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("MappedRefMut") .field("k", &self.k) @@ -319,7 +306,7 @@ impl<'a, K: Eq + Hash + Debug, V, T: Debug> Debug for MappedRefMut<'a, K, V, T> } } -impl<'a, K: Eq + Hash, V, T> Deref for MappedRefMut<'a, K, V, T> { +impl<'a, K: Eq + Hash, T> Deref for MappedRefMut<'a, K, T> { type Target = T; fn deref(&self) -> &T { @@ -327,7 +314,7 @@ impl<'a, K: Eq + Hash, V, T> Deref for MappedRefMut<'a, K, V, T> { } } -impl<'a, K: Eq + Hash, V, T> DerefMut for MappedRefMut<'a, K, V, T> { +impl<'a, K: Eq + Hash, T> DerefMut for MappedRefMut<'a, K, T> { fn deref_mut(&mut self) -> &mut T { self.value_mut() } diff --git a/src/rayon/map.rs b/src/rayon/map.rs index 92d0f690..448c46a9 100644 --- a/src/rayon/map.rs +++ b/src/rayon/map.rs @@ -1,6 +1,6 @@ -use crate::lock::RwLock; +use crate::lock::{RwLock, RwLockReadGuardDetached, RwLockWriteGuardDetached}; use crate::mapref::multiple::{RefMulti, RefMutMulti}; -use crate::{DashMap, HashMap}; +use crate::{DashMap, HashMap, Shard}; use core::hash::{BuildHasher, Hash}; use crossbeam_utils::CachePadded; use rayon::iter::plumbing::UnindexedConsumer; @@ -79,7 +79,7 @@ where } pub struct OwningIter { - pub(super) shards: Box<[CachePadded>>]>, + pub(super) shards: Box<[Shard]>, } impl ParallelIterator for OwningIter @@ -95,13 +95,7 @@ where { Vec::from(self.shards) .into_par_iter() - .flat_map_iter(|shard| { - shard - .into_inner() - .into_inner() - .into_iter() - .map(|(k, v)| (k, v.into_inner())) - }) + .flat_map_iter(|shard| shard.into_inner().into_inner().into_iter()) .drive_unindexed(consumer) } } @@ -124,7 +118,7 @@ where } pub struct Iter<'a, K, V> { - pub(super) shards: &'a [CachePadded>>], + pub(super) shards: &'a [Shard], } impl<'a, K, V> ParallelIterator for Iter<'a, K, V> @@ -140,12 +134,15 @@ where { self.shards .into_par_iter() - .flat_map_iter(|shard| unsafe { - let guard = Arc::new(shard.read()); - guard.iter().map(move |b| { + .flat_map_iter(|shard| { + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockReadGuardDetached::detach_from(shard.read()) }; + let guard = Arc::new(guard); + + data.iter().map(move |data| { let guard = Arc::clone(&guard); - let (k, v) = b.as_ref(); - RefMulti::new(guard, k, v.get()) + // Safety: The guard is still protecting the data. + unsafe { RefMulti::new(guard, data) } }) }) .drive_unindexed(consumer) @@ -198,12 +195,15 @@ where { self.shards .into_par_iter() - .flat_map_iter(|shard| unsafe { - let guard = Arc::new(shard.write()); - guard.iter().map(move |b| { + .flat_map_iter(|shard| { + // Safety: The data will not outlive the guard. + let (guard, data) = unsafe { RwLockWriteGuardDetached::detach_from(shard.write()) }; + let guard = Arc::new(guard); + + data.iter_mut().map(move |data| { let guard = Arc::clone(&guard); - let (k, v) = b.as_mut(); - RefMutMulti::new(guard, k, v.get_mut()) + // Safety: The guard is still protecting the data. + unsafe { RefMutMulti::new(guard, data) } }) }) .drive_unindexed(consumer) diff --git a/src/read_only.rs b/src/read_only.rs index dae46fca..7b18cd1b 100644 --- a/src/read_only.rs +++ b/src/read_only.rs @@ -84,25 +84,23 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: BuildHasher + Clone> ReadOnlyView let idx = self.map.determine_shard(hash as usize); + // Safety: shard index is in bounds + // and this is a read-only view. let shard = unsafe { self.map._get_read_shard(idx) }; - shard.find(hash, |(k, _v)| key == k.borrow()).map(|b| { - let (k, v) = unsafe { b.as_ref() }; - (k, v.get()) - }) + shard + .find(hash, |(k, _v)| key == k.borrow()) + .map(|(k, v)| (k, v)) } /// An iterator visiting all key-value pairs in arbitrary order. The iterator element type is `(&'a K, &'a V)`. pub fn iter(&'a self) -> impl Iterator + 'a { - unsafe { - (0..self.map._shard_count()) - .map(move |shard_i| self.map._get_read_shard(shard_i)) - .flat_map(|shard| shard.iter()) - .map(|b| { - let (k, v) = b.as_ref(); - (k, v.get()) - }) - } + (0..self.map._shard_count()) + // Safety: shard index is in bounds + // and this is a read-only view. + .map(move |shard_i| unsafe { self.map._get_read_shard(shard_i) }) + .flat_map(|shard| shard.iter()) + .map(|(k, v)| (k, v)) } /// An iterator visiting all keys in arbitrary order. The iterator element type is `&'a K`. diff --git a/src/serde.rs b/src/serde.rs index 27a7c421..eb2bf734 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -7,6 +7,7 @@ use serde::ser::{Serialize, SerializeMap, SerializeSeq, Serializer}; use serde::Deserializer; pub struct DashMapVisitor { + #[allow(clippy::type_complexity)] marker: PhantomData DashMap>, } @@ -185,11 +186,11 @@ impl<'a, K: Eq + Hash, V: Serialize> Serialize for mapref::one::RefMut<'a, K, V> serialize_impl! {} } -impl<'a, K: Eq + Hash, V, T: Serialize> Serialize for mapref::one::MappedRef<'a, K, V, T> { +impl<'a, K: Eq + Hash, T: Serialize> Serialize for mapref::one::MappedRef<'a, K, T> { serialize_impl! {} } -impl<'a, K: Eq + Hash, V, T: Serialize> Serialize for mapref::one::MappedRefMut<'a, K, V, T> { +impl<'a, K: Eq + Hash, T: Serialize> Serialize for mapref::one::MappedRefMut<'a, K, T> { serialize_impl! {} } diff --git a/src/util.rs b/src/util.rs index c5eb903e..a9103fa2 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,7 +1,10 @@ //! This module is full of hackery and dark magic. //! Either spend a day fixing it and quietly submit a PR or don't mention it to anybody. -use core::cell::UnsafeCell; use core::{mem, ptr}; +use std::marker::PhantomData; +use std::mem::ManuallyDrop; + +use lock_api::{RawRwLock, RawRwLockDowngrade, RwLockReadGuard, RwLockWriteGuard}; pub const fn ptr_size_bits() -> usize { mem::size_of::() * 8 @@ -22,69 +25,93 @@ pub fn map_in_place_2 T>((k, v): (U, &mut T), f: F) { } } -/// A simple wrapper around `T` -/// -/// This is to prevent UB when using `HashMap::get_key_value`, because -/// `HashMap` doesn't expose an api to get the key and value, where -/// the value is a `&mut T`. -/// -/// See [#10](https://github.com/xacrimon/dashmap/issues/10) for details -/// -/// This type is meant to be an implementation detail, but must be exposed due to the `Dashmap::shards` -#[repr(transparent)] -pub struct SharedValue { - value: UnsafeCell, -} - -impl Clone for SharedValue { - fn clone(&self) -> Self { - let inner = self.get().clone(); +struct AbortOnPanic; - Self { - value: UnsafeCell::new(inner), +impl Drop for AbortOnPanic { + fn drop(&mut self) { + if std::thread::panicking() { + std::process::abort() } } } -unsafe impl Send for SharedValue {} - -unsafe impl Sync for SharedValue {} +/// A [`RwLockReadGuard`], without the data +pub(crate) struct RwLockReadGuardDetached<'a, R: RawRwLock> { + lock: &'a R, + _marker: PhantomData, +} -impl SharedValue { - /// Create a new `SharedValue` - pub const fn new(value: T) -> Self { - Self { - value: UnsafeCell::new(value), +impl<'a, R: RawRwLock> Drop for RwLockReadGuardDetached<'a, R> { + fn drop(&mut self) { + unsafe { + self.lock.unlock_shared(); } } +} - /// Get a shared reference to `T` - pub fn get(&self) -> &T { - unsafe { &*self.value.get() } - } +/// A [`RwLockWriteGuard`], without the data +pub(crate) struct RwLockWriteGuardDetached<'a, R: RawRwLock> { + lock: &'a R, + _marker: PhantomData, +} - /// Get an unique reference to `T` - pub fn get_mut(&mut self) -> &mut T { - unsafe { &mut *self.value.get() } +impl<'a, R: RawRwLock> Drop for RwLockWriteGuardDetached<'a, R> { + fn drop(&mut self) { + unsafe { + self.lock.unlock_exclusive(); + } } +} - /// Unwraps the value - pub fn into_inner(self) -> T { - self.value.into_inner() +impl<'a, R: RawRwLock> RwLockReadGuardDetached<'a, R> { + /// Separates the data from the [`RwLockReadGuard`] + /// + /// # Safety + /// + /// The data must not outlive the detached guard + pub(crate) unsafe fn detach_from(guard: RwLockReadGuard<'a, R, T>) -> (Self, &'a T) { + let rwlock = RwLockReadGuard::rwlock(&ManuallyDrop::new(guard)); + + let data = unsafe { &*rwlock.data_ptr() }; + let guard = unsafe { + RwLockReadGuardDetached { + lock: rwlock.raw(), + _marker: PhantomData, + } + }; + (guard, data) } +} - /// Get a mutable raw pointer to the underlying value - pub(crate) fn as_ptr(&self) -> *mut T { - self.value.get() +impl<'a, R: RawRwLock> RwLockWriteGuardDetached<'a, R> { + /// Separates the data from the [`RwLockWriteGuard`] + /// + /// # Safety + /// + /// The data must not outlive the detached guard + pub(crate) unsafe fn detach_from(guard: RwLockWriteGuard<'a, R, T>) -> (Self, &'a mut T) { + let rwlock = RwLockWriteGuard::rwlock(&ManuallyDrop::new(guard)); + + let data = unsafe { &mut *rwlock.data_ptr() }; + let guard = unsafe { + RwLockWriteGuardDetached { + lock: rwlock.raw(), + _marker: PhantomData, + } + }; + (guard, data) } } -struct AbortOnPanic; - -impl Drop for AbortOnPanic { - fn drop(&mut self) { - if std::thread::panicking() { - std::process::abort() +impl<'a, R: RawRwLockDowngrade> RwLockWriteGuardDetached<'a, R> { + /// # Safety + /// + /// The associated data must not mut mutated after downgrading + pub(crate) unsafe fn downgrade(self) -> RwLockReadGuardDetached<'a, R> { + unsafe { self.lock.downgrade() } + RwLockReadGuardDetached { + lock: self.lock, + _marker: self._marker, } } }