Skip to content

Commit dea05e9

Browse files
authored
Remove unsound cast in thread local resources (#749)
* Remove unsound cast in thread local resources * Make ResourceRef(Mut)::new impossible to cause unsoundness with
1 parent 1d4a95d commit dea05e9

File tree

1 file changed

+43
-16
lines changed

1 file changed

+43
-16
lines changed

crates/bevy_ecs/src/resource/resources.rs

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ trait ResourceStorage: Downcast {}
3333
impl_downcast!(ResourceStorage);
3434

3535
struct StoredResource<T: 'static> {
36-
value: T,
36+
value: std::cell::UnsafeCell<T>,
3737
atomic_borrow: AtomicBorrow,
3838
}
3939

@@ -45,27 +45,24 @@ impl<T: 'static> VecResourceStorage<T> {
4545
fn get(&self, index: usize) -> Option<ResourceRef<'_, T>> {
4646
self.stored
4747
.get(index)
48-
.map(|stored| ResourceRef::new(&stored.value, &stored.atomic_borrow))
48+
.map(|stored| ResourceRef::new(stored))
4949
}
5050

5151
fn get_mut(&self, index: usize) -> Option<ResourceRefMut<'_, T>> {
52-
self.stored.get(index).map(|stored|
53-
// SAFE: ResourceRefMut ensures that this borrow is unique
54-
unsafe {
55-
let value = &stored.value as *const T as *mut T;
56-
ResourceRefMut::new(&mut *value, &stored.atomic_borrow)
57-
})
52+
self.stored
53+
.get(index)
54+
.map(|stored| ResourceRefMut::new(stored))
5855
}
5956

6057
fn push(&mut self, resource: T) {
6158
self.stored.push(StoredResource {
6259
atomic_borrow: AtomicBorrow::new(),
63-
value: resource,
60+
value: std::cell::UnsafeCell::new(resource),
6461
})
6562
}
6663

6764
fn set(&mut self, index: usize, resource: T) {
68-
self.stored[index].value = resource;
65+
self.stored[index].value = std::cell::UnsafeCell::new(resource);
6966
}
7067

7168
fn is_empty(&self) -> bool {
@@ -414,9 +411,24 @@ pub struct ResourceRef<'a, T: 'static> {
414411

415412
impl<'a, T: 'static> ResourceRef<'a, T> {
416413
/// Creates a new resource borrow
417-
pub fn new(resource: &'a T, borrow: &'a AtomicBorrow) -> Self {
418-
borrow.borrow();
419-
Self { resource, borrow }
414+
fn new(
415+
StoredResource {
416+
value,
417+
atomic_borrow,
418+
}: &'a StoredResource<T>,
419+
) -> Self {
420+
if atomic_borrow.borrow() {
421+
Self {
422+
// Safe because we acquired the lock
423+
resource: unsafe { &*value.get() },
424+
borrow: atomic_borrow,
425+
}
426+
} else {
427+
panic!(
428+
"Failed to acquire shared lock on resource: {}",
429+
std::any::type_name::<T>()
430+
);
431+
}
420432
}
421433
}
422434

@@ -454,9 +466,24 @@ pub struct ResourceRefMut<'a, T: 'static> {
454466

455467
impl<'a, T: 'static> ResourceRefMut<'a, T> {
456468
/// Creates a new entity component mutable borrow
457-
pub fn new(resource: &'a mut T, borrow: &'a AtomicBorrow) -> Self {
458-
borrow.borrow_mut();
459-
Self { resource, borrow }
469+
fn new(
470+
StoredResource {
471+
value,
472+
atomic_borrow,
473+
}: &'a StoredResource<T>,
474+
) -> Self {
475+
if atomic_borrow.borrow_mut() {
476+
Self {
477+
// Safe because we acquired the lock
478+
resource: unsafe { &mut *value.get() },
479+
borrow: atomic_borrow,
480+
}
481+
} else {
482+
panic!(
483+
"Failed to acquire exclusive lock on resource: {}",
484+
std::any::type_name::<T>()
485+
);
486+
}
460487
}
461488
}
462489

0 commit comments

Comments
 (0)