I apparently made a huge assumption by thinking any code that has a reference to the player is destroyed when the player leaves the game. I thought I was especially safe by placing code inside a function connected to PlayerAdded…I was wrong. The following code shows a memory leak that will get worse and worse as players join and leave (The use case for the loop is an auto-save feature):
local players = game:GetService("Players")
local function onPlayerAdded(player)
while true do --This will constantly run even when the player leaves!!
wait(2)
print("Never stops")
end
end
local playerConnect = players.PlayerAdded:Connect(onPlayerAdded)
I tried different ways to prevent the loop from running after the player leaves the game such as:
onPlayerAdded = nil --Tried this just in case
onPlayerAdded(5) --Will error if placed after the first one, yet the loop will still run
playerConnect:Disconnect() --Tried this just in case a reference was kept up
player:Destroy() --This will error and tell you the parent is locked
The ONLY way I can prevent the loop from running is by creating a high-scoped table keyed on the player’s name; then destroying the key like this:
local players = game:GetService("Players")
local userData = {}
local function onPlayerAdded(player)
local playerName = player.Name
userData[playerName] = true
while userData[playerName] do
wait(2)
for i, v in pairs(userData) do
print(i, v)
end
end
end
local playerConnect = players.PlayerAdded:Connect(onPlayerAdded)
players.PlayerRemoving:Connect(function(player)
userData[player.Name] = nil --Manually break the loop and clear the cache
end)
That’s great and all, but what about the local variable playerName inside the onPlayerAdded function? Does that get garbage collected? Does this mean I’ll need to create every local variable as a value in the userData[player.Name] key and manually make sure the data are garbage collected (I’m guessing I shouldn’t have to)? Or is there a better way to avoid these leaks?
[EDIT] If a server script has a remote connection in it, then there is a copy of that function for each player. Same thing with PlayerAdded, each player gets their own copy of the PlayerAdded function and any function that has the default argument of player. My assumption was that anything inside the PlayerAdded function got cleared out once the player left because it is their copy. I just wanted to clear up why I assumed this and to help anyone else thinking the same thing.
Oh, didn’t know that happens, thanks for the reminder!
Why I thought that that was the solution was because when the player leaves, the player instance gets destroyed, therefore player would be nil. In a conditional statement, nil would come across as false(not that it is false, just meaning that it will not pass the statement), therefore, breaking the loop.
Destroying is simply setting the parent to nil and clearing all connections. If there’s a strong reference it won’t be garbage collected per my understanding. So the variable will point to an instance with nil parent until all references are released (i.e. the loop stops checking it).
That does work, and I think you’re right that the player is parented to nil upon leaving the game.
After thinking about it, it’s probably better to use your method. It seems like it may cause an error with saving if I nil out the player’s key in PlayerRemoving while code is still running for that key within the loop.
There is no direct “relationship” between the infinite loop and the player instance so it won’t die. I think it might work better to not have the infinite loop in the playeradded event handler at all but to write similar code but connect a handler to the RunService.Stepped event and then you can query if the player still exists every two seconds and do what you want there instead?
I was under the impression that anything locally defined gets garbage collected when a function ends. So if I did this:
local players = game:GetService("Players")
local function onPlayerAdded(player)
local anExample = "Just an example"
while true do --This will constantly run even when the player leaves!!
local anExample2 = "Another Example"
wait(2)
print("Never stops")
end
end
local playerConnect = players.PlayerAdded:Connect(onPlayerAdded)
Then anExample will be wiped out as soon as the while true do ends; and anExample2 is cleared every time the loop hits “end”. So in other words, the while true do loop also gets wiped out as soon as the onPlayerAdded function ends because it is local to that function.
Please correct me if I’m wrong on this, but this has been my understanding of it.
[EDIT] This actually even explains why the while true do loop still fires after the player leaves the game: the function never ended. I wrongly assumed any code running inside PlayerAdded would stop when the player left the game. My original thought about the code inside PlayerAdded was that it was linked to the player instance.
Correct, it never reaches the closing end because of the infinite loop. It’s actually an event of the Players Service as well, not the player instance. So remember it’s not:
player.Added:Connect(onPlayerAdded)
So if there was any instance cleanup it would be on the destruction of the Players service - but event handlers are normally “fire and forget” in most languages, rather than referenced to an instance. I hope this makes sense!
This also means it is a problem with any infinite loop on the server that runs individually per client. Unless of course you run :GetPlayers() and run the loop for each player; then record the timestamp of when they individually joined so you don’t get a nasty lag spike when 30+ players all get auto-saved at the same time. This option actually doesn’t sound too bad
Here’s an example from my game script using the RunService.Stepped event to process logic for all game players - this won’t run because I deleted stuff and other stuff is not present but hopefully you can read through and it will give you an idea for an alternative way to do this…
local _playerTimers = {}
local function ClearPlayerGame(player)
if(player == nil) then
warn("ERROR player is nil")
return
end
local playerKey = player.UserId
_playerTimers[playerKey] = nil
end
local function OnStepped(t, dt)
local funcName = "MissionManager::OnStepped"
_oneSecondCounter = _oneSecondCounter + dt
if(_oneSecondCounter < 1) then return end
_oneSecondCounter = 0
_countedTime = _countedTime + 1
for playerKey, timer in pairs(_playerTimers) do
local player = _playerLookup:Lookup(playerKey)
if player == nil then
warn(funcName .. " ERROR player nil")
end
timer["GameTime"] = _countedTime - timer["GameStart"]
if (_playerGameEggs[playerKey] == _constants.GAME_EGGS) then
_dataStores:SetTime(player, timer["GameTime"])
ClearPlayerGame(player)
elseif(timer["GameTime"] > _constants.GAME_LENGTH_SECS) then
ClearPlayerGame(player)
if player.Character ~= nil and player.Character.Humanoid ~= nil then
player.Character.Humanoid.Health = 0
end
end
end
end
_runService.Stepped:Connect(OnStepped)
local function AddPlayerTimer(player)
local funcName = "MissionManager::AddPlayerTimer player:" .. tostring(player)
print(funcName)
local playerKey = player.UserId
_playerTimers[playerKey] = {
["GameStart"] = _countedTime,
["GameTime"] = 0
}
end
-- Called when the player triggers an event by sitting in a truck
function MissionManager:StartFirstMission(player)
AddPlayerTimer(player)
-- Missing:
--CreateNextMission(player)
end
