What's causing this leak?

Geomaster · December 30, 2019, 9:14am

I’m attempting to diagnose and prevent memory leaks in a game I’m aiding development with.
I understand that variables and connections can leak memory instances and thus must be dealt with appropriately before the garbage collector can free the memory they occupy.

However, I’ve run into an impasse, and require further guidance.

While testing exactly what causes memory leaks in our game, I’ve found a possible culprit. These training dummies are never garbage collected once they are killed and destroyed. They cause the Instances, PhysicsParts, and Gui entries of the server memory to gradually and irreversibly rise, and I attempted my best at making sure they were properly garbage-collected, to no avail.

I removed all but one script essential to their operation in order to ascertain what behaviors I need to adopt in order to ensure no leaks occur, and then proceeded to follow the instructions of the many forums I’d read as to what needs to be done to ensure no references are left behind that could cause the collector to ignore the dead dummy. However, my attempt was not successful.

function wfc(p, o, t)
	return p:WaitForChild(o, t)
end

function newC3(r, g, b)
	return Color3.new(r/255, g/255, b/255)
end

--
local v3 = Vector3.new
local c3 = Color3.new
local ud2 = UDim2.new

--
local cs = game:GetService("CollectionService")
local ss = game:GetService("ServerStorage")
local rs = game:GetService("ReplicatedStorage")

local remotes = wfc(rs, "Remotes")
local spoof = wfc(remotes, "EffectSpoof")
local dpopup = wfc(remotes, "DmgPopup")

local cModules = wfc(rs, "Modules")
local common = wfc(cModules, "CommonEffects")

local sModules = wfc(ss, "Modules")
local core = require(wfc(sModules, "Core"))

--
local overhead = wfc(rs.Assets, "HealthMeter"):Clone()
overhead.Parent = script

local o_name = wfc(overhead, "NamePlate")
local o_bar = wfc(overhead, "Bar")
local o_hp = wfc(o_bar, "HP")
local o_lay = wfc(o_bar, "Underlay")
local o_sh = wfc(o_bar, "Shield")
local o_c = wfc(o_bar, "Counter")

local char = script.Parent
local hume = wfc(char, "Humanoid")
local hrp = wfc(char, "HumanoidRootPart")
local head = wfc(char, "Head")

local sf = wfc(char, "Stats")
local hp = wfc(sf, "HP")
local mhp = wfc(sf, "MaxHP")
local shd = wfc(sf, "Shield")
local spd = wfc(sf, "Speed")
local rec = wfc(sf, "Recovery")

--
local dead = false
local cons = {}

local defwait = 4
local regwait = defwait
local maxwait = 20

local old = hrp.Position
local new = char:Clone()
local lasthit = tick()

local chp = hp.Value
local cmhp = mhp.Value
local dsp = hume.WalkSpeed
local mreg = 4

--
local function getShields(char)
	local num = shd.Value
	local sf = char:FindFirstChildWhichIsA("Folder")
	
	if sf and sf.Name:lower() == "stats" then
		for _,v in pairs(sf:GetChildren()) do
			if v.Name:lower() == "tempshield" then
				num = num + v.Value
			end
		end
	end
	
	return num
end

local function hpsync()
	local s = getShields(char)
	local val = hp.Value
	
	o_hp.Size = ud2(val/mhp.Value, 0, 1, 0); o_c.Text = val + s
	o_sh.Size = ud2(math.min(s/mhp.Value, 1), 0, 1, 0)
				
	if val <= mhp.Value * .25 then o_hp.BackgroundColor3 = newC3(255, 0, 0)
	elseif val <= mhp.Value *.6 then o_hp.BackgroundColor3 = newC3(255, 255, 0)
	else o_hp.BackgroundColor3 = newC3(50, 255, 50)
	end
	
	spawn(function()
		while (tick()-lasthit) < .6 do wait() end
		o_lay:TweenSize(o_hp.Size, "Out", "Quad", 0.4, true)
	end)
end


--
table.insert(cons, hume.Died:connect(function()
	hp.Value = 0; dead = true
end))

table.insert(cons, hp.Changed:connect(function(amt)
	local diff = chp - amt

	if amt <= 0 then dead = true; char:BreakJoints()
		delay(5, function()
			new.Parent = char.Parent
			new:MoveTo(old); new = nil
			char:Destroy()
		end)
	
	else if diff > 0 then 
			lasthit = tick(); regwait = math.min(regwait + (diff/10), maxwait) 
		end
	end 
	
	chp = amt; hpsync()
end))

table.insert(cons, mhp.Changed:connect(function(amt)
	if amt < hp.Value then hp.Value = amt
	elseif amt >= hp.Value and hp.Value == cmhp then hp.Value = amt
	end cmhp = amt
end))

table.insert(cons, spd.Changed:connect(function(val) hume.WalkSpeed = dsp * (val/100) end))

table.insert(cons, shd.Changed:connect(hpsync))
table.insert(cons, sf.ChildAdded:connect(hpsync))

table.insert(cons, hume.Touched:connect(function(part)
	if cs:HasTag(part, "Void") or cs:HasTag(part, "Water") then
		hp.Value = 0; shd.Value = 0
	end
end))

--
overhead.NamePlate.Text = char.Name
overhead.Parent = head

hpsync()

--
while char.Parent do
	if (hrp.Position - old).magnitude >= 500 then hp.Value = 0 end
	
	if not dead then local now = tick()
		repeat wait() until (tick()-now) >= .75
	
		if (tick() - lasthit) >= regwait and hp.Value < mhp.Value then
			local amt = math.ceil(mreg * (1.1 - (hp.Value/mhp.Value)))
			amt = math.clamp(amt, 2, mreg)
			amt = math.ceil(amt * (rec.Value/100))
		
			hp.Value = math.min(hp.Value + amt, mhp.Value)
			core:makePopup(head.Position + v3(0, 2, 0), "+"..amt, newC3(0, 255, 0), c3(), 1.2)
			core:effect("heal", {part = hrp, power = amt * 2}, common)
			
			regwait = defwait
		end
		
		else break
	end
end

for _,v in pairs(cons) do v:Disconnect() end

cons = nil

If anyone could peruse that code snippet and determine exactly what’s causing the leak, I’d appreciate your help. Thanks for reading.

Geomaster · December 30, 2019, 5:33pm

Update.

After reading more community resources I’ve discovered there’s a “programmatical” method to check and see if an object gets garbage-collected.

I’ve run this test on every instance belonging to the dummy, and all of them return nil after the dummy has been vaporized due to dying. Why, then, does the server memory irreversibly go up whenever a new dummy spawns in its place? It should fall back down after the old one is cleaned up, correct? No other references to the dummy exist within the game’s code. Any ideas?

colbert2677 · December 30, 2019, 5:58pm

I find that to be a fairly silly method. Correct me if I’m wrong in what I say, but Roblox instances can never be weak or so that’s the intended behaviour of referencing them in a weak table; it’s always strong. This and now you’re creating a strong reference with your script so now it becomes an inaccurate test.

As for the script itself, I’m still puzzling around what’s going on to get an understanding if where things may be leaking instances. Since Instances, PhysicsParts and Gui are supposedly experiencing leaks, that’ll mean your NPC itself as well as any associated Gui - I’m assuming, in this case, would be a health bar of some kind.

This may be a product of attempting to take shortcuts in your code, such as the gross amount of upvalues which aren’t commonly used. You may want to consider revising bits of code where possible and ensure you keep as little references as possible and destroy what you don’t need.

I can’t quite give a full answer right now since I’d still have to pick at the code. The lack of descriptive variable names makes a first glance more difficult for me to look at.

Geomaster · December 30, 2019, 6:12pm

I’ve done plenty of reading on the topic, and there are so many questions and specifics left unattended to regarding memory management that I often notice a major disconnect in what people believe to be true. Yesterday, you replied to a post of mine stating that references are lost in a script that ceases to run. I’ve seen people argue the contrary. I’ve seen people say you don’t need to set unused variables to nil, and I’ve seen some that say you do. The lack of official documentation on the subject makes for a confusing time, especially for someone who just now only realizes it is common practice in Roblox Lua to help the garbage-collector free up space.

That said, the only purpose of that code I posted is to gradually heal the dummy when damaged, and to replace it when it has died. I think the variable names are just informative enough to demonstrate how this happens.

colbert2677 · December 30, 2019, 6:19pm

Depends on who you’re asking and how versed they are with code. There are often a lot of misconceptions floating around and a lot of what you’d be able to observe is from your own testing and knowledge. Not saying I know everything though.

As for what you’ve brought up, this is also very dependent on the context of your code. If a script is not running or has otherwise completed its execution, its akin to finishing running a function. Think of the entire script encapsulated in a function, or the main method of a Java class. Properly handled references will not cause leaks. You definitely do not need to set unused variables to nil as they will be implicitly when the current scope closes (in pertains to local variables).

Geomaster · December 30, 2019, 6:20pm

So, then - do I need to mark my variables as local regardless of scope, or will global variables be treated as local to the scope of the entire script?

colbert2677 · December 30, 2019, 6:26pm

Other than your functions which are globals right now (you can change this with local function f), yes you should try to aim to write all variables as locals. I don’t think globals have any actual utility in Roblox Lua unless you are modifying function environments (don’t, they disable LuaU optimisations and aren’t necessary) or are using something like _G/shared (don’t, use ModuleScripts instead).

Global variables will not be treated as locals. In essence you can’t use them anywhere else except the current script so it may seem like they’re treated as locals as well.

idkhowtocode · December 30, 2019, 6:42pm

while char.Parent do

end

This block will never break. It seems intuitive that when char is destroyed, it’s parent should be falsey, but this is not the case.

Potential solutions

Custom variable that controls the loop which is set to false when deleting the character
Instead of char.Parent, verify that char exists by searching in workspace/wherever you put it

edit: lol sorry didn’t see you had coded in an exit condition. I would still put a few prints in to verify that it is exiting the block as intended, because it will not stop looping without hitting your break

Geomaster · December 30, 2019, 6:52pm

The loop is definitely ending - I threw in a print along with a randomly generated ID to make sure each script was stopping, and they did 100% of the time.