DenseAssetStorage<bevy_gltf::Gltf>::insert: 'internal error: entered unreachable code: entries should always be valid after a flush'.
#10820
Comments
marcelchampagne
added
C-Bug
alice-i-cecile
added
A-Assets
|
I've been getting this error as well |
|
A little more information about the error on my end (to help with debugging): First of all, my error isn't occurring for I don't think I even have bevy_gltf compiled in my game, as it's more of a TUI game. In my case, I also have custom asset loaders, including assets that depend on other assets. Secondly, I also get a second type of error, with this trace: The result is unwrapped on line 305, but the |
|
For the original issue, I think it's because you're only keeping a strong handle to the subassets ( It can probably happen every time an asset is loaded with a label and the main asset is ignored, but it seems random and load dependent. As a workaround in user code, you can keep a strong handle to the main asset (a |
|
@mockersf Thank you so much for investigating! I tried your suggested workaround and it seems to have resolved the issue (unless I've yet to encounter the panic😅). Your hypothesis makes a lot of sense though and would explain why others may not be encountering it. Still I'm somewhat surprised given that loading a GLTF Scene without the entire Gltf file is probably one of the most common things I'd expect users to do with GLTF files. But as we observed since it's load dependent but the race condition only really occurs when lots of assets are being loaded. |
|
Can confirm that the workaround fix is indeed working as I've been running with it for a couple of days without any panics. I also doubled checked and my full application code still panics maybe 1/5 times without it so @mockersf assessment of the issue is accurate at least for my case. Unrelated but GLTF assets in general take much longer to load in 0.12 than in 0.11.x. Just an FYI but loading 200 assets is taking ~8 seconds for me now and it was maybe ~1 second before, if there isn't an existing issue for that happy to file one, but it's causing iteration time to be a fair bit longer. |
|
This may have possibly been resolved by #11986 |
|
@AxiomaticSemantics I can confirm that the issue still exists on main branch |
|
For me the error didn't stop occurring until I put both the main asset and the sub-assets in static Mutex<Vec<Handle<_>>>, only the main assets or only the sub assets still produced the issue. Edit: even that doesn't solve the problem for me, it just makes it more rare. |
|
I don't suppose any progress has been made on this issue? |
alice-i-cecile
added
the
S-Needs-Investigation
|
Welp, as far as I'm reading here, this issue can be "fixed" by never doing sub-asset loads, and only loading "Handle<< Gltf >>"s, and accessing elements from there. This is how "bevy_gltf_components" loads everything, for example. Seems to be working for me so far, will report back if I happen to see this error again. |
|
I would categorize that as a workaround more than a fix, especially since
that means that you can no longer just store a handle for the asset you
want, you need to store the path to the sub asset with it.
…On Mon, May 20, 2024, 11:39 AM Aronry ***@***.***> wrote:
Welp, as far as I'm reading here, this issue can be "fixed" by never doing
sub-asset loads, and only loading "Handle"s, and accessing elements from
there. This is how "bevy_gltf_components" loads everything, for example.
Seems to be working for me so far, will report back if I happen to see this
error again.
—
Reply to this email directly, view it on GitHub
<#10820 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA7EGS34OKL7UMUJCRJHHLLZDI7OTAVCNFSM6AAAAABAB7ILNKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRQHE4TOOBVGE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
I've done some investigation into this issue, and I think I know where the problem lies, but it's a tricky one to fix. I managed to get the reproduction rate up by limiting What I think is happening is this:
I've been pondering it a bit, and I've come up with a couple of semi-sensible solutions:
Either way, I think it's probably a good idea to remove |
|
Turns out I was probably over-thinking the solution. Since the handle is an asset-server-managed, I can just check whether it's been removed from the list of asset IDs we know about, and skip inserting it into If anyone has the means and a project which triggers this issue, testing with that fix would be much appreciated. |
|
I have a project, but with the new release coming up there are bound to be other problems when I switch to targeting main/your PR. But since I don't want to wait until 0.15 for this fix, I'mma give it a shot, maybe try temporarily deleting extraneous plugins until I get a pared down version that still reproduces the error. |
|
@Testare I've backported the change to 0.13.2, to make it a bit easier to test. You should be able to do: [dependencies]
bevy = { git = "https://github.com/ricky26/bevy.git", branch = "bugfix/labelled-asset-loading-0.13" }In your Cargo.toml, if you were using bevy 0.13 before. |
|
Oh sorry didn't see this XD I appreciate the effort though! Regardless, the migration to main was rather smooth this time, and this'll probably give more accurate results anyways. So using bevyengine/bevy:main, I was able to reproduce the error after a couple runs. In fact, the error was much longer than before, which would probably be a good thing for debugging. I could include that trace here but... When I was tracking your PR, I was not able to reproduce the crash! I even had a loop something like |
|
Thanks for taking the time to check @Testare. 🙂 |
|
Thanks for taking the time to fix! I would be quite sad to wait until 0.15 for this =] |
alice-i-cecile
added
S-Needs-Review
# Objective - Fixes #10820. ## Solution - Check that the asset ID to be inserted is still being managed. - Since this route is only used by `AssetServer`-tracked handles, if the `infos` map no longer contains the asset ID, all handles must have been dropped. In this case, since nobody can be watching for the result, we're safe to bail out. This avoids the panic when inserting the asset, because when the handles are dropped, its slot in `Assets<A>` is poisoned. - Someone may be waiting for a labelled asset rather than the main asset, these are handled with separate calls to `process_asset_load`, so shouldn't cause any issues. - Removed the workaround keeping asset info alive after the handle has died, since we should no longer be trying to operate on any assets once their handles have been dropped. ## Testing - I added a `break` in `handle_internal_asset_events` (`crates/bevy_asset/src/server/mod.rs` on line 1152). I don't believe this should affect correctness, only efficiency, since it is effectively only allowing one asset event to be handled per frame. This causes examples like `animated_fox` to produce the issue fairly frequently. - I wrote a small program which called `AssetServer::reload` and could trigger it too. --- ## Changelog - Fixed an issue which could cause a panic when loading an asset which was no longer referenced. --- ## Remaining Work ~This needs more testing. I don't yet have a complete project that reliably crashes without changes to bevy.~ We have at least one vote of confidence so far from @Testare who had a project broken by this bug. @cart, (sorry for the ping), I believe you added the code which delays `remove_dropped`. Was there any other reason `track_assets` needed to keep the dropped assets alive?
Bevy version
0.12.1Relevant system information
What you did
I have a simple asset plugin that loads GLB files and inserts the handles for the scenes and meshes into a resource. In my full application, which loads approximately 200 assets, I encounter a panic immediately after running cargo run after upgrading to Bevy 0.12.1 from 0.11.3.
I've provided an MRE below. Unfortunately, with the below example, the panic only occurs, roughly, 1 out of every 50 times. In my full application, it seems to happen much more frequently, perhaps 1 out of every 10 runs.
What went wrong
Additional information
The same panic was investigated in #10444. It seems this same code is also panicking, but it appears to be unrelated to AssetEvent::Removed this time, since assets are not being removed in this case.
For debugging purpose, I've included the assets that are being loaded in the example above here: https://drive.google.com/file/d/1Nq9fBoARR6p9nxXc-aOJt2Lut5-UpZMP/view?usp=sharing. The assets are from Mixamo and Qauternius's Ultimate Animated Animal Pack.
The text was updated successfully, but these errors were encountered: