Skip to content

Potential for UB in systems using AssetChanged #23057

@Trashtalk217

Description

@Trashtalk217

Bevy version and features

08a3f24 (main)

What you did

I ran the following test in bevy_asset/src/asset_changed.rs.

#[test]
#[should_panic]
fn should_conflict() {
    // should conflict, doesn't.
    fn system(
        q1: Query<&Foo, AssetChanged<MyComponent>>,
        q2: Query<&mut AssetChanges<MyAsset>, bevy_ecs::query::Without<Foo>>,
    ) {
    }
   assert_is_system(system);
}

It failed.

What went wrong

q1, through AssetChanged places a read access on AssetChanges<MyAsset>. Meanwhile, q2 writes to AssetChanges<MyAsset>, and so the two queries should conflict. Yet they don't. This is potentially a source of UB.

Additional information

While the situation where one SystemParam tries to access both an entity and a resource is rare, this only became UB after #20934. Currently, this has zero impact, because AssetChanges<MyAsset> is pub(crate) and we never create a Query<&mut AssetChanges<A>> query. However, if we remove resources from Access, this problem can also be triggered through ResMut<AssetChanges<A>>. Such a query does exist in the bevy codebase now, and as such this issue blocks #22910.

There are several solutions.

  • The most promising is Nested Queries #21557, which allows queries to query multiple different entities.
  • Assets as entities v0 #22939 might also be a solution, but I haven't had time to look at how that redesign is going to work.
  • A very ugly solution would be to wrap AssetChanges in an Arc<RwLock<...>> somehow, so conflicts don't matter.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-AssetsLoad files from disk to use for things like images, models, and soundsA-ECSEntities, components, systems, and eventsC-BugAn unexpected or incorrect behaviorP-UnsoundA bug that results in undefined compiler behaviorS-Needs-DesignThis issue requires design work to think about how it would best be accomplished

    Type

    No type

    Projects

    Status

    Done

    Status

    Done

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions