Skip to content

Resolve memory leaks in tests reported by miri#24979

Open
chescock wants to merge 4 commits into
bevyengine:mainfrom
chescock:fix-leaks
Open

Resolve memory leaks in tests reported by miri#24979
chescock wants to merge 4 commits into
bevyengine:mainfrom
chescock:fix-leaks

Conversation

@chescock

Copy link
Copy Markdown
Contributor

Objective

Make it possible to run miri with memory leak detection enabled. It is enabled by default, so it reports errors whenever I run it locally. And running it in CI should help prevent any new leaks from being introduced.

See #4310 and #4959 for previous attempts to enable this. Note that miri has added backtraces to leaked allocations since then (rust-lang/rust#109061), so leaks are much easier to diagnose.

Solution

The first leak: When trying to drop non-send data on the wrong thread, the allocation for the value was being leaked. We can't drop the value from the wrong thread, but we could still deallocate its storage.

Rework the thread check in NonSendData::drop so that it still deallocates, setting present = false to ensure the value's drop is not called. Change the panic! to a warn! so that we can have the check work the same way during unwinding.

The second leak: The tests for Interned intentionally leak data! So we can't exactly "fix" them, but I'd like miri to stop failing on them. Then I realized that miri does not report leaks for our use of Interned for things like ScheduleLabel. That's because those values are stored in a static, so are still reachable.

So, add a static to each of the tests with intentional leaks and store the leaked values there. miri will consider those values reachable, and not report any leaks.

@alice-i-cecile alice-i-cecile added this to the 0.20 milestone Jul 13, 2026
@chescock chescock added A-ECS Entities, components, systems, and events A-Build-System Related to build systems or continuous integration C-Code-Quality A section of code that is hard to understand or change S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 13, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Jul 13, 2026
Comment thread crates/bevy_ecs/src/storage/non_send.rs
Comment thread crates/bevy_ecs/src/storage/non_send.rs Outdated
Co-authored-by: Daniel Skates <zeophlite@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Build-System Related to build systems or continuous integration A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

4 participants