The problem is when there persists a strong reference to the underlying instance through the engine. Ideally we shouldn’t need :Destroy(). I’m personally of the mindset that it’s the engine’s single most glaring design flaw; and that they really ought to stop expanding its utility—and start investigating other means of treating instances in a more GC compliant fashion.
In the beginning, we did not have :Destroy(). Afterall, the inherent advantage of landing on a garbage collected language, was that scripters would not need to bother themselves with the woes of manual memory management. Unfortunately, it soon-after became apparent that functions connected to signals could possess strong references back to the associated instance—and this cycle of references would prevent that memory from ever being considered eligible for garbage collection. Thus :Destroy() was added. At this time, with what limited utility it had, it could be seen almost as an extension of the events API. Sure, the language is garbage collected; but in the case that you happen to open a connection, either remember to explicitly close it, or call :Destroy() to sever everything bound to the instance’s signals. Undesirable; but simple enough.
In recent years however, it’s unfortunately become again apparent that there exists yet more possibilities for memory leaks within the engine. And it’s been Roblox’s (I think questionable) response, knowing full well the magnitude of this problem, to expand on the behavior of :Destroy() to likewise treat these less common edge-cases. So whereas :Destroy() was once the niche case treatment, it is now much closer to the general rule. But :Destroy() isn’t really a generic cleanup method. As is clear from its evolution, it’s really just a bundle of patchwork solutions targeted at addressing very specific pitfalls. So these unprecedented expectations are actually damaging; because they become the self-same justification for Roblox to further expand the method’s largely undocumented influence.
I agree with everything you’ve said, and it’s a shame that roblox requires us to handle the stuff it should already be handling for us. But I guess there’s nothing we can really do about it.
Where did you get that from? From what I can read, instances wont get gc if they have events connected to them. The link you provided suggests to either disconnect the events, or use :Destroy(), which disconnects the events, and deletes the instance
From what I can read, the docs don’t state that Players and Characters aren’t gc’ed, but rather that they don’t get destroyed, so leaving events connected will prevent them from getting gc’ed, and this applies to any instance
I’m not saying that you shouldn’t destroy the character and the player, I think it’s a good idea to do so as your post states (and something I should check in my own scripts), but your post isn’t quite factually accurate. Your post suggests that you will for sure have a memory leak if you don’t do so, which is not correct
It is however very common to have Player.CharacterAdding and Player.CharacterRemoving connections, that don’t get disconnected, which will cause a memory leak, as I have just read from the docs
This documentation is slightly misleading. An unsevered connection in of itself is not necessarily casual of memory leakage. As I brushed past in my last post, the problem here is when the function’s closure contains a typical reference (keep in mind directly or indirectly) back to the associated instance. And of course, in Lua/u this capture behavior is implicit.
As such:
-- Will leak
Player.CharacterAdded:Connect(function(Character)
print(`{Player.Name} has spawned!`)
end)
-- Will also leak
local Indirection = {Player}
Player.CharacterAdded:Connect(function(Character)
print(`{Indirection[1].Name} has spawned!`}
end)
-- Will not leak
Player.CharacterAdded:Connect(function(Character)
print(`{Character.Name} has spawned!`)
end)
-- Likewise will not leak
local PlayerName = Player.Name
Player.CharacterAdded:Connect(function(Character)
print(`{PlayerName} has spawned!`)
end)
Maybe it’s just me, but given your examples, is that how Roblox recommends setting up connections in documentation examples or how everyone just does this? I only ask because the “Leak” examples seem to be the wrong way to setup the connections anyway, regardless of it causing a leak.
What I mean is, I usually build a function first, then call the function via the connection like this:
local oPlayers = game:GetService("Players")
function onPlayerAdded(oPlayer)
-- Player joined server, do some stuff
end
oPlayers.PlayerAdded:Connect(onPlayerAdded)
No, this function will only run once and the event is only set up once. Taking up very little memory.
Think of memory leaks like a bath tub
The tub is the maximum amount of memory that can be stored
The tap is the thing that you can use to store memory (e.g. rbxscriptsignals, arrays)
tap water is the memory usage
Now what happens when you leave the tap on for too long? The water (memory usage) starts building up and eventually overflows, causing leaks through the ceilings and what not.
May’ve been a bad example but I hope it made sense.
Ah ok, I wanted to make sure I was understanding the issue properly. The good examples are using the object within the function, the bad examples (leak prone) would be the ones where the object that the event was connected to was being referenced directly within itself. I’ve seen a lot of other people write code that way in examples here on the forum, which I thought was just a shortcut like “counter += 1” instead of “counter = counter + 1”.
Glad I never took up that “shortcut” for my coding I guess.
The decision of whether to use a named function, or an anonymous function, is only, at most, tangentially relevant to the issue I was discussing. All examples could be made applicable in either fashion. Now, in saying this, I’ll acknowledge that organizationally, choosing to prefer named functions, may force scripters to write better code; and that might in consequence push them away from similar pitfalls. Afterall, avoiding this means of nesting altogether, can effectively stop an inexperienced scripter from unwittingly shooting themselves in the foot. But there’s nothing inherent in the distinction itself that would address the fundamental problem here. That being, if the connected function’s closure (directly, or through another object), preserves a typical reference back to the instance associated with that signal, the connection must be closed for memory to be freed. Irrespective of how the connected function was defined.
For future readers, I should note that I’m like to continue discussion on this subject; but know that I’m generally hesitant to comment on what practices I personally consider to be ‘good’ or ‘preferred’. And as such, would much rather write further only on purely technical matters.
