Potential memory leak in Rigidbody Component

[LeXXik]

When a Rigidbody Component is removed from the entity, the Component System calls its onRemove method and passes the entity and a component data object. Since rigidbody component was migrated to the new class, it dropped the reliance on the component data object. Since then onRemove method is incorrectly checking for the private body property.

entity.removeComponent('rigidbody');

pc.ComponentSystem#removeComponent(): Notice it passes a component data as a last argument. https://github.com/playcanvas/engine/blob/1967983c1cb975c2dfd2cd2232a6d4ebf50e644f/src/framework/components/system.js#L85

pc.RigidBodyComponentSystem#onRemove however thinks it is a component and tests its body property, which will always fail, since its component data is not used. https://github.com/playcanvas/engine/blob/1967983c1cb975c2dfd2cd2232a6d4ebf50e644f/src/framework/components/rigid-body/system.js#L410-L418

Consider moving it to the onBeforeRemove, which receives actual component:

[willeastcott]

Wow, this is a great observation. Yes, you're completely correct. body in onRemove will always be undefined. However, the solution is not quite as simple as you suggest. RigidBodyComponentSystem#onRemove is also called from here:

https://github.com/playcanvas/engine/blob/1967983c1cb975c2dfd2cd2232a6d4ebf50e644f/src/framework/components/rigid-body/component.js#L444

I am also wondering whether RigidBodyComponentSystem#removeBody might be called more than once if we make that change (because removeBody also gets called from RigidBodyComponent#disableSimulation).

Basically, this might require a little refactoring to fix it properly.

[ArztSamuel]

We are also affected by this issue. Would be awesome to have a dev-side workaround until it is fixed in the engine properly.

[nzmax]

We also affected by this! Hope it will be fixed recently.