Memory Leak Check

Help and Feedback Code Review
#1

Hello,

Can anyone check this for memory leaks?

local RunService = game:GetService('RunService')

local Parts = {}

function CreatePart(Connection1, Connection2, HoseDiameter, HoseColor, Parent)

	local newPart = Instance.new('Part')
	newPart.Color = HoseColor
	newPart.Material = Enum.Material.Sand
	newPart.Shape = 'Cylinder'
	newPart.Locked = true
	newPart.CanCollide = false
	newPart.Anchored = true

	local n  = HoseDiameter
	local x1 = Connection1.Position.X
	local x2 = Connection2.Position.X
	local y1 = Connection1.Position.Y
	local y2 = Connection2.Position.Y
	local z1 = Connection1.Position.Z
	local z2 = Connection2.Position.Z

	--Assuming Positive
	local L =  math.sqrt((x1-x2)^2+(z1-z2)^2)
	local Length = math.sqrt(L^2 + (y2-y1)^2)

	if x2 - x1 < 0 and z2 - z1 > 0 then
		L = -L
	end

	if x2 - x1 < 0 and z2 - z1 < 0 then
		L = -L
	end

	local Angle  = -(math.deg(math.atan((z1-z2)/(x1-x2))))
	local AngleV = -(math.deg(math.atan((y1-y2)/L)))
	local PosX   = (x1+x2)/2
	local PosY   = (y1+y2)/2
	local PosZ   = (z1+z2)/2

	if x2 - x1 == 0 then
		AngleV = -AngleV
	end

	if x2 - x1 < 0 and z2 - z1 == 0 then
		AngleV = -AngleV
	end

	if x2 - x1 == 0 and z2 - z1 == 0 then
		Angle = 90
	end

	newPart.Size = Vector3.new(Length, n, n)
	newPart.Orientation = Vector3.new(0, Angle, AngleV)
	newPart.Position = Vector3.new(PosX, PosY, PosZ)
	
	newPart.Parent = Parent

	return newPart
	
end

RunService.RenderStepped:Connect(function()
	for _, Part in pairs(Parts) do
		Part:Destroy()
	end
	for _, Hose in pairs(game.Workspace:WaitForChild('TakenHoses'):GetChildren()) do
		if Hose:FindFirstChild('Node2').Value and Hose:FindFirstChild('Connection2').Value then
			local Node = Hose.Node2.Value
			local Connector = Hose.Connection2.Value
			local Diameter = Hose.HoseDiameter.Value
			local Color = Hose.HoseColor.Value
			local Part = CreatePart(Node, Connector, Diameter, Color, Hose)
			table.insert(Parts, Part)
		end
	end
end)
#2

I’m not sure where there could be a memory leak, as it doesn’t seem as it’s communicating client/server so I don’t really know what you mean. Bear in mind, I’m not an expert on this stuff, I just believe it’s highly unlikely.

If you are quite concerned, instead of using Part:Destroy(), I tend to use :Remove() as it automatically sets it to nil and removes it safely.

#3

Hello,
I got told there would be a memory leak somewhere in hear or smth - It’s a Local script but I didn’t know if it was table related or smth?

Many Thanks for :Remove though I’ll change it now

#4

Oh okay, it just seems unlikely and I think the ROBLOX developer community gets too stressed around memory leaks and makes it sound really dramatic.

#5

memory starts at 2000 mb then when hoses memory using up to 2200 and when I put hoses back onto the truck it goes to 2100

It causes client lag heavily apparently

#6

Perhaps it’s because you never appear to be emptying the Parts table? You call :Destroy() on the parts, but never actually remove them from the array. So you have a bunch of nil parented instances that can’t be picked up by garbage collection, because the table is maintaining references to them.

1 Like
#7

This is true! clear out that parts table.

Memory leaks crash servers babe :heart:
They’re not that bad on a small scale for a short amount of time (usually), but I recall working on a game in which servers just crashed out of the blue after only a few hours of uptime. Iirc, it was caused by just one memory-leaking script.

Now for the reason why I’m writing this post:

Do not do this. Instance:Remove() is deprecated and can actually cause more memory leaks. Instance:Remove() does not disconnect connections made to events related to the instance, meaning that they’re just floating around in memory somewhere. Meanwhile, Instance:Destroy() does. Instance:Remove() also does not lock the parent property once it’s called. There is never a good reason to use Instance:Remove(). Do not use Instance:Remove(). I’m not saying this to be mean, I’m saying this because I do not want you to tank your game.

I strongly urge you to change it back to Instance:Destroy().

Do not use Instance:Remove().

:Destroy() docs
:Remove() docs

1 Like