diff --git a/packages/generational-box/src/sync.rs b/packages/generational-box/src/sync.rs index 9e816dc1b..58a627ab4 100644 --- a/packages/generational-box/src/sync.rs +++ b/packages/generational-box/src/sync.rs @@ -19,6 +19,9 @@ use crate::{ type RwLockStorageEntryRef = RwLockReadGuard<'static, StorageEntry>; type RwLockStorageEntryMut = RwLockWriteGuard<'static, StorageEntry>; +type AnyRef = MappedRwLockReadGuard<'static, Box>; +type AnyRefMut = MappedRwLockWriteGuard<'static, Box>; + pub(crate) enum RwLockStorageEntryData { Reference(GenerationalPointer), Rc(RcStorageEntry>), @@ -59,13 +62,16 @@ pub struct SyncStorage { impl SyncStorage { pub(crate) fn read( pointer: GenerationalPointer, - ) -> BorrowResult>> { - Self::get_split_ref(pointer).map(|(_, guard)| { - RwLockReadGuard::map(guard, |data| match &data.data { - RwLockStorageEntryData::Data(data) => data, - RwLockStorageEntryData::Rc(data) => &data.data, - _ => unreachable!(), - }) + ) -> BorrowResult<(AnyRef, GenerationalPointer)> { + Self::get_split_ref(pointer).map(|(resolved, guard)| { + ( + RwLockReadGuard::map(guard, |data| match &data.data { + RwLockStorageEntryData::Data(data) => data, + RwLockStorageEntryData::Rc(data) => &data.data, + _ => unreachable!(), + }), + resolved, + ) }) } @@ -99,14 +105,16 @@ impl SyncStorage { pub(crate) fn write( pointer: GenerationalPointer, - ) -> BorrowMutResult>> - { - Self::get_split_mut(pointer).map(|(_, guard)| { - RwLockWriteGuard::map(guard, |data| match &mut data.data { - RwLockStorageEntryData::Data(data) => data, - RwLockStorageEntryData::Rc(data) => &mut data.data, - _ => unreachable!(), - }) + ) -> BorrowMutResult<(AnyRefMut, GenerationalPointer)> { + Self::get_split_mut(pointer).map(|(resolved, guard)| { + ( + RwLockWriteGuard::map(guard, |data| match &mut data.data { + RwLockStorageEntryData::Data(data) => data, + RwLockStorageEntryData::Rc(data) => &mut data.data, + _ => unreachable!(), + }), + resolved, + ) }) } @@ -277,7 +285,7 @@ impl Storage for SyncStorage { fn try_read( pointer: GenerationalPointer, ) -> Result, error::BorrowError> { - let read = Self::read(pointer)?; + let (read, pointer) = Self::read(pointer)?; let read = MappedRwLockReadGuard::try_map(read, |any| { // Then try to downcast @@ -298,7 +306,7 @@ impl Storage for SyncStorage { fn try_write( pointer: GenerationalPointer, ) -> Result, error::BorrowMutError> { - let write = Self::write(pointer)?; + let (write, pointer) = Self::write(pointer)?; let write = MappedRwLockWriteGuard::try_map(write, |any| { // Then try to downcast diff --git a/packages/generational-box/src/unsync.rs b/packages/generational-box/src/unsync.rs index 770940763..2202cafd2 100644 --- a/packages/generational-box/src/unsync.rs +++ b/packages/generational-box/src/unsync.rs @@ -13,8 +13,9 @@ use std::{ }; type RefCellStorageEntryRef = Ref<'static, StorageEntry>; - type RefCellStorageEntryMut = RefMut<'static, StorageEntry>; +type AnyRef = Ref<'static, Box>; +type AnyRefMut = RefMut<'static, Box>; thread_local! { static UNSYNC_RUNTIME: RefCell> = const { RefCell::new(Vec::new()) }; @@ -54,13 +55,16 @@ pub struct UnsyncStorage { impl UnsyncStorage { pub(crate) fn read( pointer: GenerationalPointer, - ) -> BorrowResult>> { - Self::get_split_ref(pointer).map(|(_, guard)| { - Ref::map(guard, |data| match &data.data { - RefCellStorageEntryData::Data(data) => data, - RefCellStorageEntryData::Rc(data) => &data.data, - _ => unreachable!(), - }) + ) -> BorrowResult<(AnyRef, GenerationalPointer)> { + Self::get_split_ref(pointer).map(|(resolved, guard)| { + ( + Ref::map(guard, |data| match &data.data { + RefCellStorageEntryData::Data(data) => data, + RefCellStorageEntryData::Rc(data) => &data.data, + _ => unreachable!(), + }), + resolved, + ) }) } @@ -98,13 +102,16 @@ impl UnsyncStorage { pub(crate) fn write( pointer: GenerationalPointer, - ) -> BorrowMutResult>> { - Self::get_split_mut(pointer).map(|(_, guard)| { - RefMut::map(guard, |data| match &mut data.data { - RefCellStorageEntryData::Data(data) => data, - RefCellStorageEntryData::Rc(data) => &mut data.data, - _ => unreachable!(), - }) + ) -> BorrowMutResult<(AnyRefMut, GenerationalPointer)> { + Self::get_split_mut(pointer).map(|(resolved, guard)| { + ( + RefMut::map(guard, |data| match &mut data.data { + RefCellStorageEntryData::Data(data) => data, + RefCellStorageEntryData::Rc(data) => &mut data.data, + _ => unreachable!(), + }), + resolved, + ) }) } @@ -274,7 +281,7 @@ impl Storage for UnsyncStorage { fn try_read( pointer: GenerationalPointer, ) -> Result, error::BorrowError> { - let read = Self::read(pointer)?; + let (read, pointer) = Self::read(pointer)?; let ref_ = Ref::filter_map(read, |any| { // Then try to downcast @@ -295,7 +302,7 @@ impl Storage for UnsyncStorage { fn try_write( pointer: GenerationalPointer, ) -> Result, error::BorrowMutError> { - let write = Self::write(pointer)?; + let (write, pointer) = Self::write(pointer)?; let ref_mut = RefMut::filter_map(write, |any| { // Then try to downcast diff --git a/packages/generational-box/tests/errors.rs b/packages/generational-box/tests/errors.rs index c748a6100..ad02e92ea 100644 --- a/packages/generational-box/tests/errors.rs +++ b/packages/generational-box/tests/errors.rs @@ -33,6 +33,18 @@ fn create_at_location>( (value, location) } +#[track_caller] +fn create_at_location_rc>( + owner: &Owner, +) -> ( + GenerationalBox, + &'static std::panic::Location<'static>, +) { + let location = std::panic::Location::caller(); + let value = owner.insert_rc(1); + (value, location) +} + #[test] fn read_while_writing_error() { fn read_while_writing_error_test + 'static>() { @@ -54,6 +66,27 @@ fn read_while_writing_error() { read_while_writing_error_test::(); } +#[test] +fn read_while_writing_error_rc() { + fn read_while_writing_error_test + 'static>() { + let owner = S::owner(); + let value = owner.insert_rc(1); + + let (write, location) = write_at_location(value); + + assert_eq!( + value.try_read().err(), + Some(BorrowError::AlreadyBorrowedMut( + AlreadyBorrowedMutError::new(location) + )) + ); + drop(write); + } + + // For sync storage this will deadlock + read_while_writing_error_test::(); +} + #[test] fn read_after_dropped_error() { fn read_after_dropped_error_test + 'static>() { @@ -70,6 +103,22 @@ fn read_after_dropped_error() { read_after_dropped_error_test::(); } +#[test] +fn read_after_dropped_error_rc() { + fn read_after_dropped_error_test + 'static>() { + let owner = S::owner(); + let (value, location) = create_at_location_rc(&owner); + drop(owner); + assert_eq!( + value.try_read().err(), + Some(BorrowError::Dropped(ValueDroppedError::new(location))) + ); + } + + read_after_dropped_error_test::(); + read_after_dropped_error_test::(); +} + #[test] fn write_while_writing_error() { fn write_while_writing_error_test + 'static>() { @@ -96,6 +145,32 @@ fn write_while_writing_error() { write_while_writing_error_test::(); } +#[test] +fn write_while_writing_error_rc() { + fn write_while_writing_error_test + 'static>() { + let owner = S::owner(); + let value = owner.insert_rc(1); + + #[allow(unused)] + let (write, location) = write_at_location(value); + + let write_result = value.try_write(); + assert!(write_result.is_err()); + #[cfg(any(debug_assertions, feature = "debug_borrows"))] + assert_eq!( + write_result.err(), + Some(BorrowMutError::AlreadyBorrowedMut( + AlreadyBorrowedMutError::new(location) + )) + ); + + drop(write); + } + + // For sync storage this will deadlock + write_while_writing_error_test::(); +} + #[test] fn write_while_reading_error() { fn write_while_reading_error_test + 'static>() { @@ -117,3 +192,25 @@ fn write_while_reading_error() { // For sync storage this will deadlock write_while_reading_error_test::(); } + +#[test] +fn write_while_reading_error_rc() { + fn write_while_reading_error_test + 'static>() { + let owner = S::owner(); + let value = owner.insert_rc(1); + + let (read, location) = read_at_location(value); + + assert_eq!( + value.try_write().err(), + Some(BorrowMutError::AlreadyBorrowed(AlreadyBorrowedError::new( + vec![location] + ))) + ); + + drop(read); + } + + // For sync storage this will deadlock + write_while_reading_error_test::(); +}