Can someone double check this for memory leaks?

local api = {}

local function on(lamp)
	lamp.DynamicLight.Enabled = true
	lamp.Lamp.Enabled = true
end

local function off(lamp)
	lamp.DynamicLight.Enabled = false
	lamp.Lamp.Enabled = false
end

function api:analyzeSignalInputCommand(command,config,signalSet)
	if command[1] == "on" then
		local group = command[2]
		for _,trafficSignalName in pairs(group) do
			local signalObject = signalSet.Signals:FindFirstChild(trafficSignalName)
			on(signalObject.Lights:FindFirstChild(command[3]))
		end
	elseif command[1] == "off" then
		local group = command[2]
		for _,trafficSignalName in pairs(group) do
			local signalObject = signalSet.Signals:FindFirstChild(trafficSignalName)
			off(signalObject.Lights:FindFirstChild(command[3]))
		end
	elseif command[1] == "wait" then
		if tonumber(command[2]) then
			wait(command[2])
		end
	end
end

local function handleModeChange(signalSetCycles,signalSetState,newState,signalSetConfiguration,signalSet)
	local cycle = signalSetCycles[signalSetState.Value]
	for _,thread in pairs(cycle) do
		coroutine.resume(coroutine.create(function()
			while wait() do
				for _,command in pairs(thread) do
					if signalSetState.Value == newState then
						api:analyzeSignalInputCommand(command,signalSetConfiguration,signalSet)
					else
						break
					end
				end
			end
		end))
	end
end

function api:initTrafficSignals()
	local signalFolder = workspace:WaitForChild("Traffic Signals")
	
	for _,signalSet in pairs(signalFolder:GetChildren()) do
		local initializedValue = signalSet.Initialized
		local signalSetState = signalSet.State
		
		if initializedValue.Value == false then
			initializedValue.Value = true
			
			local signalSetConfiguration = require(signalSet.Configuration)
			local signalSetCycles = signalSetConfiguration.cycles
			
			handleModeChange(signalSetCycles,signalSetState,signalSetState.Value,signalSetConfiguration,signalSet)
			signalSetState.Changed:Connect(function(newState)
				handleModeChange(signalSetCycles,signalSetState,newState,signalSetConfiguration,signalSet)
			end)
		end
	end
end

return api

This code will run all of the traffic signals in my game when the :init function is called. Considering I don’t use coroutines very often, I want to make sure there is no bad practice here, specifically in the “handleModeChange” function. Can someone just look through that function and double check for any memory leaks/bad practice?

TL;DR - I want to confirm that coroutines automatically gc when the code inside of them is done, assuming that the coroutine has not been saved in a variable anywhere.

2 Likes

Don’t know about memory leaks, but instead of making two functions to enable/disable the lamp, just make one function:

local function toggle(boolean,lamp)
    lamp.DynamicLight.Enabled = boolean
    lamp.Lamp.Enabled = boolean
end

-- To enable
toggle(true)

-- To disable
toggle(false)
1 Like

Thanks, but this wasn’t entirely what i’m looking for. It is like that because in the future, the logic for both of those functions will become a bit more in depth and having them separate would be beneficial.

I suggest wrapping the

			signalSetState.Changed:Connect(function(newState)
				handleModeChange(signalSetCycles,signalSetState,newState,signalSetConfiguration,signalSet)
			end)

Inside of a _maid:GiveTask() function. Since its object that can be destroyed, and if that object is destroyed the code will be trying to run this function and since that object isn’t there it wouldn’t be garbage collected.

Don’t use a : in functions unless you’re calling self, which you aren’t, I suggest switching api:init to api.init.

1 Like

There are no memory leaks since you aren’t using any up values in connections that aren’t being disconnected. However, to keep things tidy, you should disconnect signalSetState - when it isn’t needed.

However, An up value usage like this, will result in upVal in this case would never get garbage collected, because the function holds a reference to that up value and that up value won’t be gced unless the function that holds the reference to it, is gced it self, which happens when the connection is disconnected.

local upVal = {}

event:Connect(function()
    print(upVal)
end)