Skip to content

Commit b3d5906

Browse files
committed
Fix unsoundness for propagate_recursive (#7003)
# Objective Fix #6983. ## Solution Mark the function `propagate_recursive` as unsafe, and specify the safety invariants through doc comments.
1 parent 8ad9a7c commit b3d5906

1 file changed

Lines changed: 42 additions & 20 deletions

File tree

crates/bevy_transform/src/systems.rs

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,21 +46,40 @@ pub fn propagate_transforms(
4646
changed |= children_changed;
4747

4848
for child in children.iter() {
49-
propagate_recursive(
50-
&global_transform,
51-
&transform_query,
52-
&parent_query,
53-
&children_query,
54-
entity,
55-
*child,
56-
changed,
57-
);
49+
// SAFETY:
50+
// - We may operate as if the hierarchy is consistent, since `propagate_recursive` will panic before continuing
51+
// to propagate if it encounters an entity with inconsistent parentage.
52+
// - Since each root entity is unique and the hierarchy is consistent and forest-like,
53+
// other root entities' `propagate_recursive` calls will not conflict with this one.
54+
// - Since this is the only place where `transform_query` gets used, there will be no conflicting fetches elsewhere.
55+
unsafe {
56+
propagate_recursive(
57+
&global_transform,
58+
&transform_query,
59+
&parent_query,
60+
&children_query,
61+
entity,
62+
*child,
63+
changed,
64+
);
65+
}
5866
}
5967
},
6068
);
6169
}
6270

63-
fn propagate_recursive(
71+
/// Recursively propagates the transforms for `entity` and all of its descendants.
72+
///
73+
/// # Panics
74+
///
75+
/// If `entity` or any of its descendants have a malformed hierarchy.
76+
/// The panic will occur before propagating the transforms of any malformed entities and their descendants.
77+
///
78+
/// # Safety
79+
///
80+
/// While this function is running, `unsafe_transform_query` must not have any fetches for `entity`,
81+
/// nor any of its descendants.
82+
unsafe fn propagate_recursive(
6483
parent: &GlobalTransform,
6584
unsafe_transform_query: &Query<
6685
(&Transform, Changed<Transform>, &mut GlobalTransform),
@@ -71,7 +90,6 @@ fn propagate_recursive(
7190
expected_parent: Entity,
7291
entity: Entity,
7392
mut changed: bool,
74-
// We use a result here to use the `?` operator. Ideally we'd use a try block instead
7593
) {
7694
let Ok(actual_parent) = parent_query.get(entity) else {
7795
panic!("Propagated child for {:?} has no Parent component!", entity);
@@ -126,15 +144,19 @@ fn propagate_recursive(
126144
// If our `Children` has changed, we need to recalculate everything below us
127145
changed |= changed_children;
128146
for child in children {
129-
propagate_recursive(
130-
&global_matrix,
131-
unsafe_transform_query,
132-
parent_query,
133-
children_query,
134-
entity,
135-
*child,
136-
changed,
137-
);
147+
// SAFETY: The caller guarantees that `unsafe_transform_query` will not be fetched
148+
// for any descendants of `entity`, so it is safe to call `propagate_recursive` for each child.
149+
unsafe {
150+
propagate_recursive(
151+
&global_matrix,
152+
unsafe_transform_query,
153+
parent_query,
154+
children_query,
155+
entity,
156+
*child,
157+
changed,
158+
);
159+
}
138160
}
139161
}
140162

0 commit comments

Comments
 (0)