How do I stop the memory leak in this code

Help and Feedback Code Review
, ,
#1

I have a script that uses topbarplus and I have several “Icons” that are created when the player first joins the game (the script is in StarterPlayerScripts). It works perfectly fine. However, using LuauHeap snapshots, I have noticed a memory leak from this script when the player respawns. I have tried a lot of things like using a janitor module that cleans up the button events when the player respawns.

When I comment out the player.CharacterAdded part of the code, the memory leak goes away.

I apologize in advance for the script being very long and not very readable. Anyways, here is my script:

local jMod = require(RL:WaitForChild("Libraries"):WaitForChild("Janitor"))
local myJanitor = jMod.new()

-- the "first" parameter is true when setup is called when the player first joins the server
-- it is false when setup is called in the characteradded event.
-- it serves to prevent prevent calling ":setLabel" when not needed.
local function setup(currentCharacter:Model,first:boolean)

	-- gets data from datastore
	local InitialPlayerData = RL:WaitForChild("GameRemotes"):WaitForChild("RequestDataClient"):InvokeServer()

	-- keeps requesting until it gets the data
	if not InitialPlayerData then
		repeat
			task.wait(0.5)
			InitialPlayerData = RL:WaitForChild("GameRemotes"):WaitForChild("RequestDataClient"):InvokeServer()
		until InitialPlayerData
	end
	
	
	-- toggle events
	myJanitor:Add(enableHitmarker.toggled:Connect(function(isSelected)
		if isSelected then

			player.PlayerGui.Cursor.HitMarkerEnabled.Value = true

			enableHitmarker:setLabel(`🟩  {"Hitmarker"}`)

			GameRemotes.UpdateSettings:FireServer("Hitmarker",true)

		else

			player.PlayerGui.Cursor.HitMarkerEnabled.Value = false

			enableHitmarker:setLabel(`🟥  {"Hitmarker"}`)

			GameRemotes.UpdateSettings:FireServer("Hitmarker",false)

		end
	end))
	
	myJanitor:Add(chatTips.toggled:Connect(function(isSelected)

		if isSelected then
			chatTips:setLabel(`🟩  {"Chat Tips"}`)
			currentCharacter.ChatTips.Value = true
			GameRemotes.UpdateSettings:FireServer("ChatTips",true)
		else
			chatTips:setLabel(`🟥  {"Chat Tips"}`)
			currentCharacter.ChatTips.Value = false
			GameRemotes.UpdateSettings:FireServer("ChatTips",false)
		end

	end))

	myJanitor:Add(deathTips.toggled:Connect(function(isSelected)

		if isSelected then
			deathTips:setLabel(`🟩  {"Combat Tips"}`)
			currentCharacter.DeathTips.Value = true
			GameRemotes.UpdateSettings:FireServer("DeathTips",true)
		else
			deathTips:setLabel(`🟥  {"Combat Tips"}`)
			currentCharacter.DeathTips.Value = false
			GameRemotes.UpdateSettings:FireServer("DeathTips",false)
		end

	end))
	
	myJanitor:Add(enableRCVisualizer.toggled:Connect(function(isSelected)

		if isSelected then
			enableRCVisualizer:setLabel(`🟩  {"Debug Mode"}`)
			currentCharacter.DebugMode.Value = true
			GameRemotes.UpdateSettings:FireServer("RcHitboxVisualizer",true)
		else
			enableRCVisualizer:setLabel(`🟥  {"Debug Mode"}`)
			currentCharacter.DebugMode.Value = false
			GameRemotes.UpdateSettings:FireServer("RcHitboxVisualizer",false)
		end
	end))
	
	myJanitor:Add(normalShiftlock.toggled:Connect(function(isSelected)

		if isSelected then
			normalShiftlock:setLabel(`🟩  {"Default Camera"}`)
			currentCharacter.NormalShiftlock.Value = true
			GameRemotes.UpdateSettings:FireServer("NormalShiftlock",true)
		else
			normalShiftlock:setLabel(`🟥  {"Default Camera"}`)
			currentCharacter.NormalShiftlock.Value = false
			GameRemotes.UpdateSettings:FireServer("NormalShiftlock",false)
		end
	end))
	
	myJanitor:Add(hideNameTag.toggled:Connect(function(isSelected)

		local nameTag:BillboardGui = currentCharacter:WaitForChild("Head"):WaitForChild("NameTagGui",10)

		if isSelected then
			hideNameTag:setLabel(`🟩  {"Hide nametag"}`)
			canSeeNametag = false
			nameTag.PlayerToHideFrom = player
			GameRemotes.UpdateSettings:FireServer("HideNametag",true)
		else
			hideNameTag:setLabel(`🟥  {"Hide nametag"}`)
			canSeeNametag = true
			nameTag.PlayerToHideFrom = nil
			GameRemotes.UpdateSettings:FireServer("HideNametag",false)
		end
	end))
	
   -- all this code serves to initialize the valuebases that are put into the character/playerguis on spawn.
	if InitialPlayerData.Settings.Hitmarker then
		
		if first then
			enableHitmarker:setLabel(`🟩  {"Hitmarker"}`)
			enableHitmarker:select()
		end
		
		player.PlayerGui:WaitForChild("Cursor"):WaitForChild("HitMarkerEnabled").Value = true
		
	else
		
		if first then
			enableHitmarker:setLabel(`🟥  {"Hitmarker"}`)
		end
		
		player.PlayerGui:WaitForChild("Cursor"):WaitForChild("HitMarkerEnabled").Value = false
	end

	if InitialPlayerData.Settings.RcHitboxVisualizer then
		
		if first then
			enableRCVisualizer:setLabel(`🟩  {"Debug Mode"}`)
			enableRCVisualizer:select()
		end
		
		currentCharacter:WaitForChild("DebugMode").Value = true
		
	else
		if first then
			enableRCVisualizer:setLabel(`🟥  {"Debug Mode"}`)
		end
		
		currentCharacter:WaitForChild("DebugMode").Value = false
	end
	
	if InitialPlayerData.Settings.NormalShiftlock then
		if first then
			normalShiftlock:setLabel(`🟩  {"Default Camera"}`)
			normalShiftlock:select()
		end
		currentCharacter:WaitForChild("NormalShiftlock").Value = true
		
	else
		if first then
			normalShiftlock:setLabel(`🟥  {"Default Camera"}`)
		end
		currentCharacter:WaitForChild("NormalShiftlock").Value = false
	end
	
	local nameTag:BillboardGui = currentCharacter:WaitForChild("Head"):WaitForChild("NameTagGui",10)

	if InitialPlayerData.Settings.HideNametag then
		if first then
			hideNameTag:setLabel(`🟩  {"Hide nametag"}`)
			hideNameTag:select()
		end
		
		canSeeNametag = false
		nameTag.PlayerToHideFrom = player
		
	else
		if first then
			hideNameTag:setLabel(`🟥  {"Hide nametag"}`)
		end
		canSeeNametag = true
		nameTag.PlayerToHideFrom = nil
	end
	
	if InitialPlayerData.Settings.ChatTips then
		if first then
			chatTips:setLabel(`🟩  {"Chat Tips"}`)
			chatTips:select()
		end
		currentCharacter:WaitForChild("ChatTips").Value = true
	else
		if first then
			chatTips:setLabel(`🟥  {"Chat Tips"}`)
		end
		currentCharacter:WaitForChild("ChatTips").Value = false
	end
	
	if InitialPlayerData.Settings.TipsOnDeath then
		if first then
			deathTips:setLabel(`🟩  {"Combat Tips"}`)
			deathTips:select()
		end
		currentCharacter:WaitForChild("DeathTips").Value = true
	else
		if first then
			deathTips:setLabel(`🟥  {"Combat Tips"}`)
		end
		currentCharacter:WaitForChild("DeathTips").Value = false
	end
	
	InitialPlayerData = nil
end

-- initial character setup (when they join)
local c = player.Character
if c then
	setup(c,true)
end

-- for when the player respawns
player.CharacterAdded:Connect(function(character)
	if c then
		c:Destroy()
		c = nil
	end
	
	-- clean up the old button events
	myJanitor:Cleanup()
	
	setup(character)
	
end)

Does anyone know what I’m missing that’s causing the memory leak? Am I missing a reference?

One thing I have thought of is to define the toggle events outside of the setup function (when the player joins the game). However, I am worried about references not being properly garbage collected.

Any help is appreciated!

#2

Does the WaitForChild not just hang when the player respawns? Try adding a timeout to let it pass to your retry loop.

#3

WaitForChild completes every time. The function runs from top to bottom every time it’s run.

#4

  1. Do Player data requests when the player loads initially and just listen to changes made by the server.

  2. Why do you have to create new toggled events for every character respawn? Are these BillboardGuis? If that is not the case, just set the ScreenGui property ResetOnSpawn as false and just check if humanoid health is greater than 0.

#5

If characteradded seems to be the problem, try using :once instead of connect so that after it does the function it disconnects