Skip to content

Bundle nonnull deref safety #22205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hymm wants to merge 2 commits into
base: main
Choose a base branch
from
+114 −35
Open

Bundle nonnull deref safety #22205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e3e0e3
nonnull pointer deref safety
hymm Dec 19, 2025
435ab24
spell the weird word correctly
hymm Dec 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions crates/bevy_ecs/src/bundle/insert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ impl<'w> BundleInserter<'w> {
let mut deferred_world = world.into_deferred();

if insert_mode == InsertMode::Replace {
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let archetype = archetype.as_ref();
if archetype.has_replace_observer() {
// SAFETY: the REPLACE event_key corresponds to the Replace event's type
Expand All @@ -188,11 +192,19 @@ impl<'w> BundleInserter<'w> {
}
}

let table = table.as_mut();
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let table = unsafe { table.as_mut() };

// SAFETY: Archetype gets borrowed when running the on_replace observers above,
// SAFETY:
// * Archetype gets borrowed when running the on_replace observers above,
// so this reference can only be promoted from shared to &mut down here, after they have been ran
let archetype = archetype.as_mut();
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let archetype = unsafe { archetype.as_mut() };

match archetype_move_type {
ArchetypeMoveType::SameArchetype => {
Expand Down Expand Up @@ -343,7 +355,11 @@ impl<'w> BundleInserter<'w> {
caller: MaybeLocation,
relationship_hook_mode: RelationshipHookMode,
) -> EntityLocation {
let archetype_after_insert = self.archetype_after_insert.as_ref();
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let archetype_after_insert = unsafe { self.archetype_after_insert.as_ref() };

let (new_archetype, new_location) = {
// Non-generic prelude extracted to improve compile time by minimizing monomorphized code.
Expand All @@ -360,7 +376,12 @@ impl<'w> BundleInserter<'w> {
&mut self.archetype_move_type,
);

self.bundle_info.as_ref().write_components(
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let bundle_info = unsafe { self.bundle_info.as_ref() };
bundle_info.write_components(
table,
sparse_sets,
archetype_after_insert,
Expand Down
98 changes: 74 additions & 24 deletions crates/bevy_ecs/src/bundle/remove.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,20 @@ impl<'w> BundleRemover<'w> {
// SAFETY: We only keep access to archetype/bundle data.
let mut deferred_world = self.world.into_deferred();
let bundle_components_in_archetype = || {
self.bundle_info
.as_ref()
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let (bundle_info, old_archetype) =
(self.bundle_info.as_ref(), self.old_archetype.as_ref());
bundle_info
.iter_explicit_components()
.filter(|component_id| self.old_archetype.as_ref().contains(*component_id))
.filter(|component_id| old_archetype.contains(*component_id))
};
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
if self.old_archetype.as_ref().has_replace_observer() {
let components = bundle_components_in_archetype().collect::<Vec<_>>();
// SAFETY: the REPLACE event_key corresponds to the Replace event's type
Expand All @@ -160,12 +169,20 @@ impl<'w> BundleRemover<'w> {
);
}
deferred_world.trigger_on_replace(
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
self.old_archetype.as_ref(),
entity,
bundle_components_in_archetype(),
caller,
self.relationship_hook_mode,
);
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
if self.old_archetype.as_ref().has_remove_observer() {
let components = bundle_components_in_archetype().collect::<Vec<_>>();
// SAFETY: the REMOVE event_key corresponds to the Remove event's type
Expand All @@ -179,6 +196,10 @@ impl<'w> BundleRemover<'w> {
);
}
deferred_world.trigger_on_remove(
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
self.old_archetype.as_ref(),
entity,
bundle_components_in_archetype(),
Expand All @@ -196,18 +217,34 @@ impl<'w> BundleRemover<'w> {
// SAFETY: There is no conflicting access for this scope.
.map(|(old, _)| unsafe { &mut *old.as_ptr() }),
&world.components,
self.bundle_info.as_ref().explicit_components(),
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
unsafe { self.bundle_info.as_ref() }.explicit_components(),
);

// Handle sparse set removes
for component_id in self.bundle_info.as_ref().iter_explicit_components() {
if self.old_archetype.as_ref().contains(component_id) {
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
for component_id in unsafe { self.bundle_info.as_ref() }.iter_explicit_components() {
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
if unsafe { self.old_archetype.as_ref() }.contains(component_id) {
world.removed_components.write(component_id, entity);

// Make sure to drop components stored in sparse sets.
// Dense components are dropped later in `move_to_and_drop_missing_unchecked`.
if let Some(StorageType::SparseSet) =
self.old_archetype.as_ref().get_storage_type(component_id)
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
unsafe { self.old_archetype.as_ref() }.get_storage_type(component_id)
{
world
.storages
Expand All @@ -222,10 +259,12 @@ impl<'w> BundleRemover<'w> {
}

// Handle archetype change
let remove_result = self
.old_archetype
.as_mut()
.swap_remove(location.archetype_row);
let remove_result =
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
unsafe { self.old_archetype.as_mut() }.swap_remove(location.archetype_row);
// if an entity was moved into this entity's archetype row, update its archetype row
if let Some(swapped_entity) = remove_result.swapped_entity {
let swapped_location = world.entities.get_spawned(swapped_entity).unwrap();
Expand All @@ -244,28 +283,36 @@ impl<'w> BundleRemover<'w> {
// Handle table change
let new_location = if let Some((mut old_table, mut new_table)) = self.old_and_new_table {
let move_result = if needs_drop {
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let old_table = unsafe { old_table.as_mut() };

// SAFETY: old_table_row exists
unsafe {
old_table
.as_mut()
.move_to_and_drop_missing_unchecked(location.table_row, new_table.as_mut())
}
} else {
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let (old_table, new_table) = unsafe { (old_table.as_mut(), new_table.as_mut()) };
// SAFETY: old_table_row exists
unsafe {
old_table.as_mut().move_to_and_forget_missing_unchecked(
location.table_row,
new_table.as_mut(),
)
old_table.move_to_and_forget_missing_unchecked(location.table_row, new_table)
}
};

// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let new_archetype = unsafe { self.new_archetype.as_mut() };
// SAFETY: move_result.new_row is a valid position in new_archetype's table
let new_location = unsafe {
self.new_archetype
.as_mut()
.allocate(entity, move_result.new_row)
};
let new_location = unsafe { new_archetype.allocate(entity, move_result.new_row) };

// if an entity was moved into this entity's table row, update its table row
if let Some(swapped_entity) = move_result.swapped_entity {
Expand All @@ -286,10 +333,13 @@ impl<'w> BundleRemover<'w> {

new_location
} else {
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let new_archetype = unsafe { self.new_archetype.as_mut() };
// The tables are the same
self.new_archetype
.as_mut()
.allocate(entity, location.table_row)
new_archetype.allocate(entity, location.table_row)
};

// SAFETY: The entity is valid and has been moved to the new location already.
Expand Down
20 changes: 14 additions & 6 deletions crates/bevy_ecs/src/bundle/spawner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,17 @@ impl<'w> BundleSpawner<'w> {
bundle: MovingPtr<'_, T>,
caller: MaybeLocation,
) -> EntityLocation {
// SAFETY: We do not make any structural changes to the archetype graph through self.world so these pointers always remain valid
let bundle_info = self.bundle_info.as_ref();
// SAFETY:
// * Pointer was created from a reference in `Self::new_with_id` and so is `dereferenceable`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let bundle_info = unsafe { self.bundle_info.as_ref() };
let location = {
let table = self.table.as_mut();
let archetype = self.archetype.as_mut();
// SAFETY:
// * Pointers are dereferenceable because they were created from a reference in `Self::new_with_id`.
// * `Self`'s lifetime is tied to an exclusive reference to `World` and it does not make structural
// changes to the world, so the data is valid for the lifetime of `Self`
let (table, archetype) = unsafe { (self.table.as_mut(), self.archetype.as_mut()) };

// SAFETY: Mutable references do not alias and will be dropped after this block
let (sparse_sets, entities) = {
Expand Down Expand Up @@ -126,8 +132,10 @@ impl<'w> BundleSpawner<'w> {

// SAFETY: We have no outstanding mutable references to world as they were dropped
let mut deferred_world = unsafe { self.world.into_deferred() };
// SAFETY: `DeferredWorld` cannot provide mutable access to `Archetypes`.
let archetype = self.archetype.as_ref();
// SAFETY:
// * dereferenceable because it was created from an exclusive reference in `Self::new_with_id`.
// * `DeferredWorld` does not provide mutable access to `Archetypes`, so it is safe to hold a reference to an archetype.
let archetype = unsafe { self.archetype.as_ref() };
// SAFETY: All components in the bundle are guaranteed to exist in the World
// as they must be initialized before creating the BundleInfo.
unsafe {
Expand Down