PhysicsImpostor.registerOnPhysicsCollide crashes engine if collider.object.dispose() called inside the callback function

Hey everyone, Haven’t seen ya for a while.

So, this is a common action when you are trying to dispose object after collision is done, for example to destroy a bullet or usable item or something else.

Expected result: object disposed in the callback and remove from the scene
Actual result: whole engine crashes I believe, even browser tab freezes, can’t do anything.
Playground reproducable demo (even crash playground): https://playground.babylonjs.com/#YQ75Z2#5

Playground working demo (without dispose inside the callback): https://playground.babylonjs.com/#YQ75Z2#4

Video how it crashes: dispose bug - YouTube (click until it crashes, sometimes it takes 10 shots or more, sometimes it crashes from the first one).

Work-around: if you use setTimeout(() => object.dispose(), 1000) inside the callback function it works fine, but this shouldn’t be expected. (At least if it is expected it should be documented, maybe).

Engine versions reproduced on:
5.5.5
4.2.2
4.2.1

1 Like

I wouldn’t be surprised that all sorts of mayhem breaks out when you do what you described. Think about the chain of events:

  1. Physics engine raises collision event, includes the colliding objects in the broadcast to subscribers.

  2. Your callback is (one of) the methods the event is dispatched to, which then proceeds to yank the collider’s object out from under it.

  3. (In the case of a crash) subsequent observers or physics engine logic blow up catastrophically

  4. (No crash) got lucky, dispose happened after everyone expecting it had already accessed it

Why does the setTimeout work (and btw you could use a zero timeout and it would work the same)? Because you’re allowing the single-threaded execution to cleanly break out of the event handler scope.

May not be 100% accurate, but in general care should be taken when disposing objects in the context of a callback!

HTH

1 Like

I understand the sequence of actions called as well as web is a single threaded usually, but the problem it should be worked around on the engine level. Take a look at any other engines, the end-point coder shouldn’t think about underlying events when he simply wants to destroy object on collision.

I don’t think it’s a case to share the same behavior implemented right on the other engines, but if you want I will.

As well, BabylonJS also not allowing to destroy collided object the same way, even timeout doesn’t help, because of something.

It shouldn’t be expected that you cannot call engine function inside engine’s callback

@RaananW might have a trick for this

I am not sure if I am doing something wrong, but it does look like it is working when I run the playground.
The spheres are disposed correctly, the playground works well.
What’s the best way to reproduce the issue?

Adding a setTimeout to the dispose will also make sure the physics forces will pass to the larger spheres - test | Babylon.js Playground (babylonjs.com)

Have you looked into or tried flagging the objects for disposal in the callback? You would follow up with an onAfterPhysicsXObservable subscriber that loops through the list of objects flagged to dispose in the previous step and calls their respective dispose methods

HTH

Colleagues,

Obviously there are lots of workarounds like nested timeouts, but this is not the case engine should behave, at least it should throw adequate exception, not binary symbols. Something like “collision callback overflowed”.

As well as it also has lots of issues with PhysicImpostor of Mesh type, because the same case which works with box are crashed when switched to mesh. I will share it later.

Those are not workarounds. If you dispose on impact, the impact will be avoided as well. You are disposing everything on impact, including the mesh’s physics impostor. In what scenario would you need to dispose an impostor upon impact, instead of, well, letting it impact the scene?

There are more babylon-style ways to deal with it (i.e. - use a timer based on the onBeforeRender observable, as an example), and avoid using setTimeout, which always feels like a bit of a “hack”. But sometimes it is a very acceptable solution.

Will be happy to see other issues you have, I hope it’ll be possible to help you solve the issues there!

1 Like

What about checking in _isImpostorPairInContact() if either of the impostors is disposed then return false like below to avoid the error? Then can dispose the mesh in the collision callback with abandon without triggering that nasty runtime error. For example if when shooting an enemy mesh and want the bullet to disappear when it hits it, without a physics impact/response… There are workarounds but still better to make the engine more robust and easier to use?


if (impostorA.isDisposed || impostorB.isDisposed) {
    return false;
}

For example here is the playground with that fix for testing/reference.