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

In example parallel_query close window cause panic #17532

Open
jiangheng90 opened this issue Jan 25, 2025 · 0 comments · May be fixed by #17533
Open

In example parallel_query close window cause panic #17532

jiangheng90 opened this issue Jan 25, 2025 · 0 comments · May be fixed by #17533
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@jiangheng90
Copy link

jiangheng90 commented Jan 25, 2025

Bevy version

0.15.1

2025-01-25T08:15:34.071475Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows 11 Home", kernel: "26100", cpu: "AMD Ryzen 7 5800 8-Core Processor", core_count: "8", memory: "15.9 GiB" }
2025-01-25T08:15:34.508783Z  INFO bevy_render::renderer: AdapterInfo { name: "NVIDIA GeForce RTX 3060 Ti", vendor: 4318, device: 9353, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "566.36", backend: Vulkan }

What you did

In example parallel_query build&run example then close window cause panic. I have tested both on windows and macos. I found this issue will happen on windows.

this panic is caused by window resource has been recycle but bounce_system is still trying to get and then cause panic.

// Bounce sprites outside the window
fn bounce_system(window: Single<&Window>, mut sprites: Query<(&Transform, &mut Velocity)>) {
    let width = window.width();
    let height = window.height();
    let left = width / -2.0;
    let right = width / 2.0;
    let bottom = height / -2.0;
    let top = height / 2.0;
    // The default batch size can also be overridden.
    // In this case a batch size of 32 is chosen to limit the overhead of
    // ParallelIterator, since negating a vector is very inexpensive.
    sprites
        .par_iter_mut()
        .batching_strategy(BatchingStrategy::fixed(32))
        .for_each(|(transform, mut v)| {
            if !(left < transform.translation.x
                && transform.translation.x < right
                && bottom < transform.translation.y
                && transform.translation.y < top)
            {
                // For simplicity, just reverse the velocity; don't use realistic bounces
                v.0 = -v.0;
            }
        });
}

Fixed in #17533

so I'm not sure this is a proper solution. but unregister all system first then recycle window maybe not easy on all platfoms

@jiangheng90 jiangheng90 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jan 25, 2025
@jiangheng90 jiangheng90 linked a pull request Jan 25, 2025 that will close this issue
@Selene-Amanita Selene-Amanita added C-Examples An addition or correction to our examples A-Windowing Platform-agnostic interface layer to run your app in and removed S-Needs-Triage This issue needs to be labelled labels Jan 25, 2025
@BenjaminBrienen BenjaminBrienen added S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants