Skip to content

Commit 62c1812

Browse files
authored
Shorten the 'world lifetime returned from QueryLens::query(). (#17694)
# Objective Fix unsoundness introduced by #15858. `QueryLens::query()` would hand out a `Query` with the full `'w` lifetime, and the new `_inner` methods would let the results outlive the `Query`. This could be used to create aliasing mutable references, like ```rust fn bad<'w>(mut lens: QueryLens<'w, EntityMut>, entity: Entity) { let one: EntityMut<'w> = lens.query().get_inner(entity).unwrap(); let two: EntityMut<'w> = lens.query().get_inner(entity).unwrap(); assert!(one.entity() == two.entity()); } ``` Fixes #17693 ## Solution Restrict the `'world` lifetime in the `Query` returned by `QueryLens::query()` to `'_`, the lifetime of the borrow of the `QueryLens`. The model here is that `Query<'w, 's, D, F>` and `QueryLens<'w, D, F>` have permission to access their components for the lifetime `'w`. So going from `&'a mut QueryLens<'w>` to `Query<'w, 'a>` would borrow the permission only for the `'a` lifetime, but incorrectly give it out for the full `'w` lifetime. To handle any cases where users were calling `get_inner()` or `iter_inner()` on the `Query` and expecting the full `'w` lifetime, we introduce a new `QueryLens::query_inner()` method. This is only valid for `ReadOnlyQueryData`, so it may safely hand out a copy of the permission for the full `'w` lifetime. Since `get_inner()` and `iter_inner()` were only valid on `ReadOnlyQueryData` prior to #15858, that should cover any uses that relied on the longer lifetime. ## Migration Guide Users of `QueryLens::query()` who were calling `get_inner()` or `iter_inner()` will need to replace the call with `QueryLens::query_inner()`.
1 parent 5ff7062 commit 62c1812

File tree

3 files changed

+46
-3
lines changed

3 files changed

+46
-3
lines changed
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
use bevy_ecs::prelude::*;
2+
use bevy_ecs::system::SystemState;
3+
4+
#[derive(Component, Eq, PartialEq, Debug)]
5+
struct Foo(u32);
6+
7+
fn main() {
8+
let mut world = World::default();
9+
let e = world.spawn(Foo(10_u32)).id();
10+
11+
let mut system_state = SystemState::<Query<&mut Foo>>::new(&mut world);
12+
{
13+
let mut query = system_state.get_mut(&mut world);
14+
let mut lens = query.as_query_lens();
15+
dbg!("hi");
16+
{
17+
let mut data: Mut<Foo> = lens.query().get_inner(e).unwrap();
18+
let mut data2: Mut<Foo> = lens.query().get_inner(e).unwrap();
19+
//~^ E0499
20+
assert_eq!(&mut *data, &mut *data2); // oops UB
21+
}
22+
dbg!("bye");
23+
}
24+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
error[E0499]: cannot borrow `lens` as mutable more than once at a time
2+
--> tests/ui\query_lens_lifetime_safety.rs:18:39
3+
|
4+
17 | let mut data: Mut<Foo> = lens.query().get_inner(e).unwrap();
5+
| ---- first mutable borrow occurs here

crates/bevy_ecs/src/system/query.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2127,7 +2127,21 @@ pub struct QueryLens<'w, Q: QueryData, F: QueryFilter = ()> {
21272127

21282128
impl<'w, Q: QueryData, F: QueryFilter> QueryLens<'w, Q, F> {
21292129
/// Create a [`Query`] from the underlying [`QueryState`].
2130-
pub fn query(&mut self) -> Query<'w, '_, Q, F> {
2130+
pub fn query(&mut self) -> Query<'_, '_, Q, F> {
2131+
Query {
2132+
world: self.world,
2133+
state: &self.state,
2134+
last_run: self.last_run,
2135+
this_run: self.this_run,
2136+
}
2137+
}
2138+
}
2139+
2140+
impl<'w, Q: ReadOnlyQueryData, F: QueryFilter> QueryLens<'w, Q, F> {
2141+
/// Create a [`Query`] from the underlying [`QueryState`].
2142+
/// This returns results with the actual "inner" world lifetime,
2143+
/// so it may only be used with read-only queries to prevent mutable aliasing.
2144+
pub fn query_inner(&self) -> Query<'w, '_, Q, F> {
21312145
Query {
21322146
world: self.world,
21332147
state: &self.state,
@@ -2138,9 +2152,9 @@ impl<'w, Q: QueryData, F: QueryFilter> QueryLens<'w, Q, F> {
21382152
}
21392153

21402154
impl<'w, 's, Q: QueryData, F: QueryFilter> From<&'s mut QueryLens<'w, Q, F>>
2141-
for Query<'w, 's, Q, F>
2155+
for Query<'s, 's, Q, F>
21422156
{
2143-
fn from(value: &'s mut QueryLens<'w, Q, F>) -> Query<'w, 's, Q, F> {
2157+
fn from(value: &'s mut QueryLens<'w, Q, F>) -> Query<'s, 's, Q, F> {
21442158
value.query()
21452159
}
21462160
}

0 commit comments

Comments
 (0)