Yeah, I stopped using :remove() as soon as I found out about the difference between the 2.
1 Like
The first one still leaks memory (Probably… if the Player object is Destroy’ed then neither would leak but I don’t think it is). You actually have to disconnect the reference to the connection that you save in the table, just saving the reference to it alone doesn’t accomplish anything:
for name, connection in pairs(playerStuffTable[plr]) do
connection:disconnect()
end
playerStuffTable[plr] = nil
The second version will work since the Destroy disconnects the connection.
A better way to accomplish the same thing would be to put all of the stats inside of a “Stats” Folder and just Destroy the whole Folder so you don’t have to loop over each individual stat.
7 Likes
For a function like this which is just in a main game script and can never be disconnected,
,would I have to dispose of the “plr” and “itemName” variables somehow? Or is only the connection to the CraftedItem event what matters?
As long as the two arguments aren’t being stored elsewhere they should be GC’d automatically (plr, itemName). So for your question, yes only the event matters because it has a reference and is connected to be listened too.
I require clarification on this. I still destroy my top-level object so in essence everything below is fine, but this gears more towards permanent scripts.
- Assign table to variable “Tweens” in a do-end block
- Insert Tweens into those tables via
table.insert - Call Destroy on the Tween once it fires
Tween.Completed
I called destroy on the tween, but I print #Tweens and it still reads 3. Are these hanging references that I need to dispose of or will the do-end block take care of that for me?
1 Like
This depends on if your Tweens table is inside or outside of the do end block and if Tweens gets referred to by any other part of the script.
This, for example, is fine, but Tween will only be garbage-collected up once the outer do end block is done:
do
local Tweens = {}
do
local Tween = MakeTween()
table.insert(Tweens, Tween)
Tween.Completed:Connect(function()
Tween:Destroy()
end)
end -- `Tween` loses one of its references, the `Tween` variable, but it still has another in the `Tweens` table, so it is not garbage-collected
end -- `Tweens` loses its only reference, so it is garbage-collected, so it no longer holds any reference to `Tween`, so `Tween` loses its only reference, so it is garbage-collected
If you want Tween garbage-collected before that (for example, when the tween is completed), then you need to manually remove the reference from inside the Tweens table:
do
local Tweens = {}
do
local Tween = MakeTween()
table.insert(Tweens, Tween)
Tween.Completed:Connect(function()
Tween:Destroy()
for Index, Value in next, Tweens do
if Value == Tween then
table.remove(Tweens, Index) -- remove reference inside `Tweens`
break
end
end
end)
end
end
Or, alternatively, you could use a dictionary. This is easier and perfect for this use case:
do
local Tweens = {}
do
local Tween = MakeTween()
Tweens[Tween] = true
Tween.Completed:Connect(function()
Tween:Destroy()
Tweens[Tween] = nil
end)
end
end
The thing to take away from this is that the tween will not be garbage-collected until all references – including the reference in the Tweens table – are gone. You either need to manually remove that reference or wait until the Tweens table gets garbage-collected itself.
8 Likes
My current code looks like the first block of code you posted. Thanks for the answer!
1 Like
If I reuse the same variable, would that remove the old reference?
For example, would this cause memory leak?
local tween = nil
v.MouseButton1Down:Connect(function()
if tween then
tween:Cancel()
end
tween = tweenService:Create(...)
tween:Play()
end)
Yes.
-- Create a table that has weak values
-- (Values in the table can be garbage collected.)
local refCounter = {}
setmetatable(refCounter, {__mode = "v"})
-- Thing to be garbage collected. In this case, a Part.
local testPart = Instance.new("Part")
-- Insert it into refCounter, the size of refCounter is now 1.
table.insert(refCounter, testPart )
print(#refCounter) -- prints 1
-- Reuse the testPart variable. testPart is now a WedgePart
testPart = Instance.new("WedgePart")
-- Wait a short while for the next garbage collection cycle
wait(3)
-- The size of refCounter is now 0, which means that
-- the old testPart was garbage collected.
print(#refCounter) -- prints 0
2 Likes
A couple questions:
- Will “:ClearAllChildren” work similar to Destroy()ing every child, or should I manually Destroy() each descendant?
- Is it bad to have a connection to an event that will always exist? For example, a ClickDetector. I won’t have to worry about stuff like that, correct?
- Do I have to worry about connections to instances that are part of the player object? For example, leaderstats values?
- What about the Player object itself, should I disconnect all Chatted events and the like?
- Should I disconnect any events in a player’s character? For example, Humanoid.ChildAdded or custom Humanoid.Died events, and so on? (Or alternatively, will :LoadCharacter() call Destroy() on a character model before loading it?)
- ClearAllChildren should Destroy all parts, so it should be fine
- Not bad to have connections that always exist. This is common for many things in a game.
- No, they are destroyed along with the Player object when the player leaves
- No, Player object is destroyed, thus disconnecting all events
- Again no; any object that gets destroyed will automatically disconnect events
If Roblox is handling the removal of items, assume that it is calling Destroy. If it’s not, that’s a bug and should be fixed.
4 Likes
Alright, just what I wanted to hear. I did change it so that it destroys the humanoid and character right before calling LoadCharacter, since it seemed like UntrackedMemory was going up if I didn’t. I didn’t test it too much though, so it could just be me interpreting the results wrong, or it being some other problem. I’ll try doing more detailed tests later.
Anyways, old servers get UntrackedMemory up to around 1 GB, so a memory leak is definitely likely. Are there any other things I should look for, aside from reviewing all of my connections?
Question: I have a lot of code that looks like this
local connection; connection = BlaBlaBla.Event:connect(function()
-- bla bla bla
if something then
connection:disconnect()
end
end)
This leaks the connection, right? Is the easiest way to solve this just to connection = nil after disconnecting?
connection will stick around for as long as some code that refers to it can be ran.
Assuming your code looks something like this:
do
local connection; connection = BlaBlaBla.Event:connect(function()
-- bla bla bla
if something then
connection:disconnect()
end
end)
end
then the only code that references connection is the function. The function there can only be ran as long as the signal is connected. Once it’s disconnected, the function has no references to it any more and is collected. Because the function no longer exists, there are no more references to connection, and it gets collected. Small cause-and-effect chart:
- signal is fired
- signal is disconnected
- now-disconnected function is collected since it now has no references
- connection is collected since it now has no references
In short:
It does not, assuming your only reference to connection is in the function and the only reference to the function is the event.
7 Likes
Always amazed at how smart Lua’s garbage collector is.
1 Like
If referencing instances from connected functions can cause serious memory problems, why aren’t the instances simply passed as arguments to their connected functions? Like this:
part.Touched:Connect(function(otherPart, part)
print(part)
end)This would also provide huge performance gains when connecting the same function to many parts:
local function onTouched(otherPart, part)
print(part)
end
for i = 1, #parts do -- Connect to many parts
parts[i].Touched:Connect(onTouched)
endConnected functions could use less than half as much memory due to fewer upvalues
4 Likes
Late reply, but, that doesn’t really solve the problem. It could solve the problem for the case of exactly solitary Roblox instances, but as soon as you had any sort of non-trivial data structures in your Lua code you’d end up back with the same issues.
You also can’t bodge around it by letting users “connect with” an additional userdata parameter that will get passed to the handler when it is invoked, like:
event:connect(function(args, userdata) ... end, userdata).
If you did you have exactly the same circular reference problem because the C++ side is holding a reference to that userdata but still doesn’t know anything about the contents of the userdata or what Roblox instances it may be referencing.
3 Likes
Say I have an event for when my player receives more xp on the server, and I want to check to see if they have enough xp to level up. The IntValue that is stored inside the player fires an event when the value is updated. So when the player leaves, all the events connected to that IntValue will disconnect right?
Yes, because when a player leaves or gets kicked, Roblox deletes the player instance. So unless you were just calling Remove() or setting player instances to nil, you should be good.
1 Like