Issue:
After parts are destroyed by: workspace.FallenPartsDestroyHeight they do not have connected signals disconnected. (I suspect it Remove's them and does not Destroy as it should.) This causes a memory leak in a few games I develop for and I suspect multiple other games as this shouldn’t be intended behavior. I believe this also does not disconnect signals to the descendants of the instance as well.
If this is patched this will make it so I do not have to implement a weird hacky detection to disconnect all signals to instances if they get removed from falling out of the world.
Expected Behavior:
When BaseParts reach the FallenPartsDestroyHeight they should be properly Destroy'ed and disconnect all connected signals to the instance, as well as, its descendants.
Repro Place:
(The example game is open-source, you may open it in Roblox Studio.) Example Game
Reproduction:
Attach a Touched, ChildAdded, ChildRemoved, or etc signal to a BasePart. Have the BasePart reach the FallenPartsDestroyHeight and be removed. Notice the signal not getting automatically disconnected.
Other Cases:
Both of these games I develop for have tools you can drop, the tools have Touched signals to detect someone picking them up, if they happen to somehow fall out of the map, they remain in memory forever with a memory leak. Game Link Game Link
Yeah fallen parts only trigger Parent = nil. This is a know issue and we don’t have any current plans to change this.
It’s worth noting that replicated removals never Destroy either. Even if you Destroy on the server, clients will only see a Parent = nil without detaching signals.
A bit unfortunate but this effectively forces you to clean up all signal connections when the instance removed from the DataModel as a general practice.
Is there a technical reason for not having any reason to change it? Is it for backwards compatibility with older games? If its an backwards compatibility thing would it be feasible to have it behind a setting under Workspace like other recent features? I think most developers would also expect for objects to be destroyed when they reach the fallen parts, especially since it includes Destroy in the name.
Having to manually call Destroy on objects leaving Workspace is not ideal for some effects games might use, e.g: Re-parenting an instance under certain conditions from between nil and a character part.
Yeah, the main reason is backwards compatibility, the same reason we don’t Destroy instances when they stream out in StreamingEnabled. We know from analytics that a fair number of games add removed instances back on clients in general, but don’t have analytics for use after fallen & removed specifically yet. Of course changing this would break that.
There is some work being considered to actually replicate Destroy from servers to clients. No ETA and and I can’t make promises for another team though. It would initially be opt-in, but likely all-in in the future.
We could do a similar thing with fallen parts, but without the above change this doesn’t seem as useful, possibly giving people a false confidence that references will be cleaned up, except for on clients where Destroy remains meaningless. Paired with the above another opt-in option could be useful here. We just don’t have plans for this yet!
Destroy also can’t delete references in Lua, so Lua references as keys or values in tables, etc. will still need to be cleaned up before the instance can be released from memory. Destroy doesn’t remove the need to clean up after the object when it is being removed. It’s easy to miss this unless you can guarantee you only have a single code path where this object can ever be destroyed, which is often not the case. It doesn’t completely remove your duty to clean up references. It just helps a little bit.
Documentation
For performance reasons, Roblox automatically destroys (using Instance:Destroy) parts that fall below this value. This is to prevent parts that have fallen off the map from continuing to fall forever.
If a part destroyed due to this behavior is the last part in a model, then that model will also be destroyed. This applies to all model ancestors of the part.
If the documentation could be updated and if making a Workspace property toggle for this could potentially be looked into, it would help a multitude of games. Also I kind of doubt anyone would rely on behavior like this which isn’t intended, documented nor a good case to do so over a memory leak. Analytics for this could potentially help for figuring out if anyone does use this behavior, but Roblox always says to not rely on behavior that is not intended. If no one does use this behavior it could easily be changed without affecting anyone’s games.
I’ve spoken with a few other developers and most didn’t even knew of this and quite a few are affected by this. All of them thought that parts were properly cleaned up because the documentation said Instances were destroyed. As one of them said, “It’s one thing if its not documented and being relied on which is just dumb, it’s another thing if the documentation is lying.”
Thank you for your time on helping explain and looking into this issue for us.
I had no clue from the longest time destroyed instances not always disconnect signals assigned to it. Why is this so poorly communicated? Documentation say nothing about that. I found this thread by complete accident. I’m pretty sure most top games on roblox experience memory leaks beacuse of that. I’ve noticed strong memory leak playing mad city for example, the game was unplayable after 3 hours on a single server. The solution that @Camstcha provided to let it be a toggleable option under workspace is brilliant. I hope we can see some improvement in the core area of roblox!
Is this a valid way to clean up object connections?
workspace.DescendantRemoving:Connect(function(obj)
task.wait()
if not obj.Parent then obj:Destroy() end
end)
or
workspace.AncestryChanged:Connect(function(obj, parent)
if not parent then obj:Destroy() end
end)
However for Workspace.FallenPartsDestroyHeight I can’t think of a single usecase which relies on it not calling :Destroy(). Besides the documentation literally says it internally :Destroy()s it.
