Skip to content
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.

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

MeshAllocator panics when spawning and despawning lots of meshes #14540

Closed
eero-lehtinen opened this issue Jul 30, 2024 · 3 comments · Fixed by #14560
Closed

MeshAllocator panics when spawning and despawning lots of meshes #14540

eero-lehtinen opened this issue Jul 30, 2024 · 3 comments · Fixed by #14560
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash S-Needs-Design This issue requires design work to think about how it would best be accomplished
Milestone

Comments

@eero-lehtinen
Copy link
Contributor

eero-lehtinen commented Jul 30, 2024

Bevy version

Main (ba09f35)

MeshAllocator was added recently in #14257.

Relevant system information

Rust v1.80.0
SystemInfo { os: "Linux rolling EndeavourOS", kernel: "6.10.1-arch1-1", cpu: "AMD Ryzen 7 5800X3D 8-Core Processor", core_count: "8", memory: "31.3 GiB" }
AdapterInfo { name: "NVIDIA GeForce RTX 3070 Ti", vendor: 4318, device: 9346, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "555.58.02", backend: Vulkan }

What you did

Ran into this issue in my game. Was able to reproduce in a fairly minimal example. After running it for a couple of seconds, it panics.

Below are my logs when running the example code and the example code itself.

Panic Logs

2024-07-30T18:34:41.538405Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Linux rolling EndeavourOS", kernel: "6.10.1-arch1-1", cpu: "AMD Ryzen 7 5800X3D 8-Core Processor", core_count: "8", memory: "31.3 GiB" }
2024-07-30T18:34:41.742583Z  INFO bevy_render::renderer: AdapterInfo { name: "NVIDIA GeForce RTX 3070 Ti", vendor: 4318, device: 9346, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "555.58.02", backend: Vulkan }
2024-07-30T18:34:42.278247Z  INFO gilrs_core::platform::platform::gamepad: Gamepad /dev/input/event9 (Generic X-Box pad) connected.    
2024-07-30T18:34:42.379764Z  INFO bevy_winit::system: Creating new window "App" (Entity { index: 0, generation: 1 })
2024-07-30T18:34:42.380248Z  INFO winit::platform_impl::linux::x11::window: Guessed window scale factor: 1.25
2024-07-30T18:34:42.385865Z  INFO bevy_input::gamepad: Gamepad { id: 0 } Connected
2024-07-30T18:34:42.686618Z  WARN bevy_render::view::window: Couldn't get swap chain texture. This often happens with the NVIDIA drivers on Linux. It can be safely ignored.
2024-07-30T18:34:44.077495Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:45.561625Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:47.125133Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:48.576740Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:50.082296Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:51.547897Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:52.980272Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:54.472600Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:56.042392Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:34:58.284618Z  INFO 3d_shapes: Freed entities
2024-07-30T18:35:00.384228Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:35:01.903551Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:35:03.328986Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:35:04.864563Z  INFO 3d_shapes: Spawned 100 entities
2024-07-30T18:35:06.239920Z  INFO 3d_shapes: Spawned 100 entities
thread 'Compute Task Pool (7)' panicked at crates/bevy_render/src/mesh/allocator.rs:674:21:
internal error: entered unreachable code: Slab not found
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_render::mesh::allocator::allocate_and_free_meshes`!
2024-07-30T18:35:07.681254Z  INFO 3d_shapes: Spawned 100 entities
thread '<unnamed>' panicked at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/thread/local.rs:260:26:
cannot access a Thread Local Storage value during or after destruction: AccessError
fatal runtime error: failed to initiate panic, error 5

Example Code

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, update)
        .run();
}

/// A marker component for our shapes so we can query them separately from the ground plane
#[derive(Component)]
struct Shape;

fn setup(mut commands: Commands) {
    commands.spawn(Camera3dBundle {
        transform: Transform::from_xyz(0.0, 7., 14.0).looking_at(Vec3::new(0., 1., 0.), Vec3::Y),
        ..default()
    });
}

fn update(
    query: Query<Entity, With<Shape>>,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
    mut commands: Commands,
    mut counter: Local<u32>,
) {
    *counter += 1;

    if *counter % 100 == 0 {
        for entity in query.iter() {
            commands.entity(entity).despawn();
        }
        info!("Freed entities");
    }

    let n = if *counter % 100 > 50 && *counter % 100 < 60 {
        100
    } else {
        0
    };

    let material = materials.add(StandardMaterial::default());
    for _ in 0..n {
        let mesh = meshes.add(Sphere::new(1.).mesh().ico(50 + n / 10).unwrap());

        commands.spawn((
            PbrBundle {
                mesh,
                material: material.clone(),
                transform: Transform::from_xyz(1000., 2., 2.),
                ..default()
            },
            Shape,
        ));
    }

    if n > 0 {
        info!("Spawned {} entities", n);
    }
}

Additional information

This change to the problem area seems to fix the issue, but I don't really understand the whole system that well so might be wrong.

diff --git a/crates/bevy_render/src/mesh/allocator.rs b/crates/bevy_render/src/mesh/allocator.rs
index 218e19c47..1599f75da 100644
--- a/crates/bevy_render/src/mesh/allocator.rs
+++ b/crates/bevy_render/src/mesh/allocator.rs
@@ -671,7 +671,7 @@ impl MeshAllocator {
         'slab: for &slab_id in &*candidate_slabs {
             loop {
                 let Some(Slab::General(ref mut slab)) = self.slabs.get_mut(&slab_id) else {
-                    unreachable!("Slab not found")
+                    continue 'slab;
                 };
 
                 if let Some(allocation) = slab.allocator.allocate(data_slot_count) {
@eero-lehtinen eero-lehtinen added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jul 30, 2024
@alice-i-cecile alice-i-cecile added P-Crash A sudden unexpected crash A-Rendering Drawing game state to the screen S-Needs-Design This issue requires design work to think about how it would best be accomplished and removed S-Needs-Triage This issue needs to be labelled labels Jul 31, 2024
@alice-i-cecile
Copy link
Member

With that fix, are all of the meshes ultimately spawned, or are some of them missing? We should be putting them back into a queue if we can't access the slab allocator there.

@eero-lehtinen
Copy link
Contributor Author

With that fix, are all of the meshes ultimately spawned, or are some of them missing? We should be putting them back into a queue if we can't access the slab allocator there.

With the fix a new slab is allocated in the failure case later in the function. The whole function:

fn allocate_general(
&mut self,
mesh_id: &AssetId<Mesh>,
data_slot_count: u32,
layout: ElementLayout,
slabs_to_grow: &mut SlabsToReallocate,
settings: &MeshAllocatorSettings,
) {
let candidate_slabs = self.slab_layouts.entry(layout).or_default();
// Loop through the slabs that accept elements of the appropriate type
// and try to allocate the mesh inside them. We go with the first one
// that succeeds.
let mut mesh_allocation = None;
'slab: for &slab_id in &*candidate_slabs {
loop {
let Some(Slab::General(ref mut slab)) = self.slabs.get_mut(&slab_id) else {
unreachable!("Slab not found")
};
if let Some(allocation) = slab.allocator.allocate(data_slot_count) {
mesh_allocation = Some(MeshAllocation {
slab_id,
slab_allocation: SlabAllocation {
allocation,
slot_count: data_slot_count,
},
});
break 'slab;
}
// Try to grow the slab. If this fails, the slab is full; go on
// to the next slab.
match slab.try_grow(settings) {
Ok(new_mesh_allocation_records) => {
slabs_to_grow.insert(slab_id, new_mesh_allocation_records);
}
Err(()) => continue 'slab,
}
}
}
// If we still have no allocation, make a new slab.
if mesh_allocation.is_none() {
let new_slab_id = self.next_slab_id;
self.next_slab_id.0 += 1;
let new_slab = GeneralSlab::new(
new_slab_id,
&mut mesh_allocation,
settings,
layout,
data_slot_count,
);
self.slabs.insert(new_slab_id, Slab::General(new_slab));
candidate_slabs.push(new_slab_id);
slabs_to_grow.insert(new_slab_id, SlabToReallocate::default());
}
let mesh_allocation = mesh_allocation.expect("Should have been able to allocate");
// Mark the allocation as pending. Don't copy it in just yet; further
// meshes loaded this frame may result in its final allocation location
// changing.
if let Some(Slab::General(ref mut general_slab)) =
self.slabs.get_mut(&mesh_allocation.slab_id)
{
general_slab
.pending_allocations
.insert(*mesh_id, mesh_allocation.slab_allocation);
};
self.record_allocation(mesh_id, mesh_allocation.slab_id, layout.class);
}

All meshes show up correctly. Though I'm not sure if it's somehow less efficient compared to what's intended.

The bug seems to be that in some cases the self.slab_layouts are not reset after freeing a slab, so we hit the unreachable.

@alice-i-cecile
Copy link
Member

Got it. Can you open a PR with the draft fix? It'll be easier to examine in that form.

@JMS55 JMS55 added this to the 0.15 milestone Aug 16, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 16, 2024
# Objective

 Fixes #14540

## Solution

- Clean slab layouts from stale `SlabId`s when freeing meshes
- Technically performance requirements of freeing now increase based on
the number of existing meshes, but maybe it doesn't matter too much in
practice
- This was the case before this PR too, but it's technically possible to
free and allocate 2^32 times and overflow with `SlabId`s and cause
incorrect behavior. It looks like new meshes would then override old
ones.

## Testing

- Tested in `loading_screen` example and tapping keyboard 1 and 2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior P-Crash A sudden unexpected crash S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants