Crash in GltfPipeline when loading two viewers with models

[OmarShehata]

Reported on the forum. If you create two Cesium viewers, each with glTF models loaded in, you get this crash:

DeveloperError: sub-region exceeds array bounds.
Error
    at new DeveloperError (http://localhost:8080/Source/Core/DeveloperError.js:43:19)
    at getStringFromTypedArray (http://localhost:8080/Source/Core/getStringFromTypedArray.js:29:19)
    at http://localhost:8080/Source/Scene/Model.js:1601:30
    at Function.ForEach.object (http://localhost:8080/Source/ThirdParty/GltfPipeline/ForEach.js:47:29)
    at Function.ForEach.shader (http://localhost:8080/Source/ThirdParty/GltfPipeline/ForEach.js:342:28)
    at parseShaders (http://localhost:8080/Source/Scene/Model.js:1593:17)
    at Model.update (http://localhost:8080/Source/Scene/Model.js:4428:25)

Sandcastle

This only happens when the second viewer is not created immediately. So it seems to be some kind of race condition?

It's throwing this error because during model load, the buffer.extras._pipeline.source array has a byte length of 0.

It's certainly a less common use case, but given that we do have an official example of using two viewers I thought it was worth documenting.

[benjaminmur]

Any ideas ?

[hpinkos]

@benjaminmur no sorry, we haven't had a chance to investigate this yet. My guess is some object is being shared between the two viewers when it shouldn't be. If you have time to look into it, we would be happy to review a pull request! Otherwise I'm not sure how soon we'll be able to prioritize this.

[mramato]

This sounds like a pretty bad regression, do we know what version it popped up in?

We should probably tag this next release.

[hpinkos]

@mramato I wasn't sure if this ever worked, but it looks like it works in 1.49 but not in 1.50

@lilleyse do you know what changed between those releases? I'm labeling this next-release since it is a regression

[lilleyse]

We upgraded to the 2.0 version of gltf-pipeline then, so it makes sense that if something broke it would be during that release.

[benjaminmur]

Thank you for making this a priority !

It worked in 1.49 and we built some main features out of this capability.
If you think i can help in any way, let me know !

[hpinkos]

Also reported by @gunmiosb in #7766

See #7766 for a Sandcastle example

[mramato]

The Sandcastle example had issues of it's own, so I cleaned up a reduce standalone test case: http://localhost:8080/Apps/Sandcastle/#c=zVjbcuI4EP0VFy/DVHmMMYSQa60wyWxmSchCkq2pOA/CFqCJsShZhmRT+fdt2TgYX7gku6n1k9V9unXULbUuM8yVGSVzwpUTxSNzxSQ+DSbaXSgrf7HDpsk8galH+BdVebE8BT7qDVmLPSmHyhC7PlEjqU9cYgvKvAvPoTYWjKcBY+ywuQ9SwYOlkAWugzw6wYIsVJb3+vXI8mZv/IytCBqfztDyhoEXelRsTkB9yRzilgPuqsqY0NFYqIsRfI2ZyUFNmU9Do5N4RCbmAv6wV9OGnE3aZMQJ8cvfqkZN0/fr9Ub1QFXqdU3f02v7eiP2LjnETscEO9QbLX1eYjHWBOuBGHt+uVrbS8KnVNhjAOsJGWeumxKNp6nJ8XvUzbU07wG+vOhXjTyqoZNkR4xTAglaHe8NB0pDxie+Nk45/DOAOHIP4OU4TqqkEcU79gouqXgGh1F4tbBNia9hxykvQi0/D09kzmRGlsK3+B++/Sa0ScKHyVYCM5F5Bm2iJ/kFnGY6C+EUZk8wuaZPxO3TvyWjqtFMg/CTBPVt7EqAocO3RLxGv69voY3GLTi2H4lzFocjigtgXmW0ZKQiqicxVauEfZ8Iq7RC3iqNCKwguSSkxiqZ3U4HtZHxvXNzbpXUJHJGuA/RiHCGplulBTU17sG3wZnU66siX8rul75S0bNKHnBNg+JPXxU9ZELz8NZZnptMX/aYug4nXkF31VR3atoeqgGnT0XWmq7mDOFTpd8+TuL/MAzgUJx5dU2CJ8Qfh5NwzUyRmM1TZcrpBKrErHBuvmRFi7UmYJIMAhFZFsAi6FW3d4k6ElZV1+Guu/2Lm4vulUQa+cBXtYgPlRtfREYvBMmKIRH1YoQs0RS7qfCmwly8dmRVjurHZZikNRnCNvD1Gd+YpEEwHBIujwZ5owP9syDd4XBR+7J6m02mzIP6efM8Dcntweabgwq80EGtkVMRisqBUdtYTai3Zd3LmIoFX6vUN1EH9VbCqW4dseq/ErFGccSM+g4Rq+5Qj7IVYofwFlTJNeLt83F3Ztb+o2wYzean5kPX9tQdpB/IR77PNeIP5COzHSzq2uYdYcAvicCuS+0eC0ZjONsUVXjIHPaJyVzGz7G9OGDd51dWXWvq4VetHhj6gXGgFuH0nTWZmZsXrXhXjIa25KunjV/X1PQecdZGeTnTtyzq2xT0vf1GLqJDvJEIzwH7RnZqYD6KzGv1g0Ztp5X6vl1mlVIR6T6cGqI9uLqJtLEx0JuDvMKpUc9WFrjXRKl1sMCHeDp15fUZLgEVZgsivvkCbr+TIznRG3UVJb8LVHlve9Rprqi2b5voep7U7Npe+Gs339k20Szlf7d2Mh5m5T3t0F8Uj7B9nWqH+nZK396Az/WX7s/cyt9Sbxb7W8NvlmffzuOTP95ZYX/mVvrrwvGG7ZacyGgOiUVnsnGO0E/UmqPvCN1K4Q+EGDLn6A+EHuGaja4QqqP2HHUR0sEC9VDrAp3NUR+1zqTxHWr9ROdz9Bdq3aK84goX/qP4xv/LD59afvS7VxqsTOqN6PC5HL4DxM9qA5cNFu86Lfgt30ubB1V5kZulXOjJNS51VuntSS7g8jlhDlcJNtduex0tevrqDn4RW0C7LJ1HrzWZR7E92Ntgh4ofxY42QgyJKamlY188u+Q0Gu1vFA46XEgmZU2rCDKZuuDErwwC+5EIzfZ9aXZciY2OHTpTqHMCp5/VJ0OrpCghCFTRk9phXdencAo6Pa6A0TpbA4y3tnVZ+MrVnRHu4mcJGVdPO5FQ07TjCjSzVoIxd4B5wuM/

This also only appears to be a problem for untextured models (at least groundvehicle loaded fine if I use that instead)

[benjaminmur]

Yes i confirm.

If you load a second viewer, with no timer, but with an untextured model, it crashes too !

Or, with timer and textured models.

[mramato]

Looking at the original Sandcastle example @OmarShehata has in his issue description, these look like two separate but related bugs. Not sure if it will be the same root cause but we definitely need to check both cases when we fix this.

[OmarShehata]

This is a glTF caching issue. The note in this Model.js comment almost predicts this issue:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Scene/Model.js#L178-L189

To demonstrate that this is the issue, just add this line in Model.js:1393:

cacheKey += String(Math.random());

This fixes both issues presented above. When GltfPipeline loads a model, it actually edits the glTF object itself (for example here https://github.com/AnalyticalGraphicsInc/gltf-pipeline/blob/master/lib/addDefaults.js#L118). Two viewers loading the same model will interfere and this will manifest in various ways. To fix this, we could:

1. Make the cache context specific, not global
2. Keep the cache global, but have some kind of flag to simulate a mutex on the glTF?

I think 2 would be better, would use less memory, and would likely load faster. Assuming it's not too hard to have Model.js wait until the glTF being loaded finishes loading, since it's already set up to load resources across multiple frames.

[mramato]

Thanks for tracking this down @OmarShehata! 1 is definitely safer and probably easier, 2 looks trivial at a glance, but I wonder if it will just cause another edge case in the future. I don't have a strong preference either way.

[lilleyse]

One fix is to not remove pipeline extras. This is what we did in 1.49 as well for probably the same reason.

@mramato's Sandcastle works now. @OmarShehata's has lighting differences though

[lilleyse]

The lighting differences are fixed in #7796.

[cesium-concierge]

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/d/msg/cesium-dev/MjFBDuFwF28/AZGHfV15AwAJ

If this issue affects any of these threads, please post a comment like the following:

The issue at #7688 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

[benjaminmur]

Thank you very much !

I've just try with the 1.57 Cesium version and it works just fine !

Thank you so much for solving this issue !