AssetContainer.dispose throws TypeError: r.metadata is null

New in 5.6.0:

Uncaught (in promise) TypeError: r.metadata is null
loadNodeAsync glTFLoader.ts:808
notifyObservers observable.ts:364
dispose node.ts:887
dispose transformNode.ts:1453
dispose abstractMesh.ts:2117
dispose mesh.ts:2455
dispose node.ts:876
dispose transformNode.ts:1453
dispose node.ts:876
dispose transformNode.ts:1453
dispose node.ts:876
dispose transformNode.ts:1453
dispose node.ts:876
dispose transformNode.ts:1453
dispose node.ts:876
dispose transformNode.ts:1453
dispose node.ts:876
dispose transformNode.ts:1453
dispose node.ts:876
dispose transformNode.ts:1453
dispose abstractMesh.ts:2117
dispose mesh.ts:2455
dispose node.ts:876
dispose transformNode.ts:1453
dispose abstractMesh.ts:2117
dispose mesh.ts:2455
dispose assetContainer.ts:518
dispose assetContainer.ts:517
… and the rest is my code. But it should not matter, right?

pinging @bghgary

@Josip_Almasi Do you have a repro in a playground? I think I know what the problem is, but it would be good to verify against a repro.

I will also note that I have repro’d the same with 5.6.0, and it appears to happen when I dispose of the root mesh (meshes[0]) of a gtlf scene. Other meshes can be disposed of just fine (meshes[1] in my case).

I’ll see if I can get a playground set up for it momentarily.

here is a repro playground: https://playground.babylonjs.com/#1Y0CSS#1

Notes what is a problem, and also noted that some glbs do not have this issue on dispose of root mesh! Very interesting indeed.

Ahh, this is different than what I thought it was. :slight_smile:
Will have a fix shortly.

PR: Fix a bug in glTF loader when loading skins by bghgary · Pull Request #12526 · BabylonJS/Babylon.js (github.com)

Thanks for the report and repro!

2 Likes

yep, I figured that would be what could fix it. Good old null checks :slight_smile:
Still odd that the line just above there doesn’t error out on line 807, yet by the time the mesh is disposed that metadata is missing. Not sure how that happens, but I’m not very familiar with Babylon code myself.

Before I found this thread, I was debugging this same issue. For reference, here is a minimal playground repro with a mesh that exhibits the problem.

I recognize that a fix is already out for review, but here’s a layman’s analysis from someone who is reading the code for the first time: In the repro, the mesh called spring has a child BouncerArmature, which has 3 children, two of which are called Bouncer. The first of these is babylonTransformNodeForSkin (aka bTNFS) and the second is babylonTransformNode (aka bTN) as mentioned on this line. After that line, both nodes share the same metadata object.

During dispose, bTNFS is being disposed first, which clears its metadata property. When bTN is later disposed, the callback on line 808 triggers, which tries to delete bTNFS.metadata.gltf.skinnedMesh and hits the null pointer. Skipping this delete means that bTN.metadata will also be set to null. Then, if there happen to be no more references to gltf, everything will be GC’d, and the delete was moot in the first place. If that’s the case, I’d consider removing this extra callback completely.

Assuming that the delete is important to preserve, then other possible fixes are to either:

  1. Set the onDisposeObservable callback on bTNFS instead of bTN, so that this delete statement executes before metadata is nulled out.

  2. Change the callback to delete bTN.metadata.gltf.skinnedMesh. That would be functionally the same, because of the copy of the metadata link earlier.

  3. Save the gltf object in a closure-captured local and delete it directly inside onDisposeObservable, ignoring the node objects and metadata fields altogether.

Of these, I think 1 or 3 are the better choices. 1 because bTNFS is the original “owner” of metadata, or 3 because it eliminates the ownership concern completely.

Apologies if I’m way off base here. Just trying to help! :smile:

1 Like

PR: More fix for glTF loader skinning code by bghgary · Pull Request #12538 · BabylonJS/Babylon.js (github.com)

Turns out that copy of metadata is not right to begin with. The gltf.pointer metadata will also be lost. I changed it to remove the copy. We now have a link to the other mesh, so other metadata from loader extensions can be retrieved from there.

2 Likes