Should I destroy players/characters on removing? Concerned about memory leaks and alive instances

Currently, I’m using @Quenty’s Maid pattern to manage connections to a player and their character respectively. This mainly stems from the supposed fear that the current behaviour for exiting players and characters does not destroy them, thus making the instances not get garbage collected. The Lua Chat System has this in place too - it holds changed connections on the Player in a table and disconnects them when a player leaves.

I’m wondering - could I not just avoid this by destroying the instances when they leave? Destroy automatically disconnects connections so as long as I’m also removing any references to either the player or their character, Destroy would handle the rest for me.

Player.CharacterRemoving:Connect(function (character)
    character:Destroy()
end)

Players.PlayerRemoving:Connect(function (player)
    player:Destroy()
end)

Assume that I’m running processes before calling Destroy. Unless it’s relevant, edge cases and race conditions don’t matter to me, since I use neither the player or their character as dependencies (e.g. putting data values in them, which is a bad idea).

Another question that could just destroy this thread entirely: am I wrong in assuming players or their characters aren’t destroyed on removing? If so, why are there complaints of connection memory leaks for players/characters or still-live instances? Why does Roblox code manually manage player connections?

Last time I tested this connected was still true on an event bound to the player 5 seconds after they left. The instances that are children of the player instance also are still there, so its safe to assume that it is not destroyed. This is probably also true for characters.

You could probably test whether you’re leaking a player/character instance with a weak table, but for a guaranteed method of disconnecting all connections it would be easy to just destroy it when its no longer needed.

So, to answer the question, the only real benefit would be simplicity. Destroying the character/player instance when no longer needed is a very simple way to eliminate possibilities of the instance sticking around forever, far simpler than implementing something to hold connections and later disconnect them.

For instance, to destroy the player instance when PlayerRemoving fires

Players.PlayerRemoving:Connect(game.Destroy)

This is far simpler to implement than having to add all connections to some table which then later is removed and all the connections it contained disconnected.

I’m writing some tests right now to confirm this, I don’t know how successful they’re going to turn out though. Obviously I can’t check if the player instance is alive nor its children because that presents a strong reference, so by nature the object isn’t going to get garbage collected.

A discussion in a certain Discord yesterday (which you were part of) makes me not want to use this. Unless you have a different repro than what was offered, I’m not willing to bank on the unexpected behaviour of weak tables and Roblox instances.

Conventionally weird, wouldn’t do. I have code in the OP for this. I’m not looking for simplicity either, that’s subjective between implementations. My main point of concern is whether or not the player and/or the character get properly destroyed with all their connections cleaned up. I’m still going to continue to use Maid for management.

Destroying the instance vs manually disconnecting the connections seems like a preference. You shouldn’t see a benefit from either method of disconnecting the events.

Instances are userdatas, and userdatas work with weak tables. The mostly confusing part is how the instance being under datamodel doesn’t actually prevent it being removed from the weak table, it needs some other strong reference.

Just making one post to compile my research and findings. If anyone sees anything incorrect or misinterpreted, please inform me. I have no clue if there are bugs in my test already. I’m going off of what I believe or know is correct, as is typical when approaching something.


I created a repro to test if my connections would stay alive after the player left. Surely enough, the connection (and by assumed extent, the player) are kept alive after a bit of time. I don’t believe the Player is destroyed though and if it is, then Destroy isn’t called immediately.

Repro details

Code:

local A

game.Players.PlayerAdded:Connect(function (Player)
	A = Player.Changed:Connect(function () end)
end)

while true do
	print(A)
	if A and typeof(A) == "RBXScriptConnection" then
		warn(A.Connected)
	end
	wait(1)
end

Results:
image

The connection is still connected (prints true from RBXScriptConnection.Connected) a bit after the player leaves, meaning that the connection lives and the Player isn’t destroyed. However, after around 12 iterations of my loop, it stopped printing true and printed false. I don’t explicitly disconnect my event, meaning the Player does get destroyed after a certain time period and thus my connection disconnects.

I’m concerned about how this may leak memory, but considering I have almost no references anywhere, my Player might be getting garbage collected appropriately. Destroy only prepares an instance for garbage collection, important tab to remember.

To summarise and conclude: the above repro leads me to believe that manual connection management is sort of a side thing that can be done, especially if I have references inside such connections. Destroying would also probably work but I don’t want to bank on that. It already looks like Destroy is called internally.

I can’t quite figure out what’s the deal with complaints about player instances and connections being kept alive or leaking memory though. Realistically if Destroy is being called, that shouldn’t be an issue. I’ll have to look more into the behaviour of removed players, references and connections.


I had a look at this thread:

And I feel like I can get an answer from here, but I don’t know what I’m processing. I’m not a big technical guy, I only really know stuff about Lua on the surface and that’s as far as I go.


In the OP and some times before, I mentioned that Roblox code manages connections manually. If Roblox code is doing this, it could potentially debunk my finding about players being destroyed. Clearly they aren’t if Roblox does manual connection management too. As well, I found out that there’s a reference to the player in the function, meaning all the more reason to manage connections.

Going to take this for now. Thanks @Muoshuu.

If roblox allows you to destroy the character and player just do that

It is very difficult and a waste of your time to keep track of each individual connection, and you are bound to make mistakes

————
When making that thread I was not aware Roblox allowed you to call Destroy on a player instance (and i was also not aware characters aren’t destroyed). I’m a little excited now because if we are allowed to destroy manually, all of my game’s memory leak problems will disappear, and I won’t have to wait the infinite time it will take roblox to fix the bug

I actually do track all of these connections using maids, and make sure they disconnect. I’m fairly certain even if this does memory leak you should be safe, because player join and leave are relatively infrequent.

3 Likes