:LoadCharacter() leaks memory if you don't manually :Destroy() the old character

Am I wrong in thinking that calling Player.Character:Destroy() should automatically nil out Player.Character?
Because if so, the behavior should be identical if you nil manually or not.
But the behavior does actually change.

1 Like

Hello.
Just wanted to mention that we are looking at introducing a mode where the engine will automatically call Destroy on players that are leaving and on characters that are being unloaded (after all the custom PlayerRemoving/CharacterRemoving callbacks are fired).

Until that is ready, we recommend manually calling Destroy.
We are also looking at ensuring that default character scripts disconnect any connection they make, without requiring a Destroy call.

42 Likes

hi
Do you have an example code that doesn’t leak memory?
Destroying a character still leaks memory it seems…

3 Likes

You might have a different issue, you should open a separate topic with a description of your case.

4 Likes

This has been a headache for a while now, it’s great to see it finally being fixed!

Would it be possible to look into these as well? These make LoadCharacter() dangerous to use, which results in developers needing to call them in a separate pseudothread to avoid hangs. They were reported a while ago but never actioned on:

3 Likes

First destroy the character then set it equal to nil

plr.Character:Destroy()
plr.Character = nil

This disconnects all connections and destroys the character’s children

1 Like

There are multiple reasons why LoadCharacter might error, so I don’t see what we can improve aside from documenting that pcall is required for error handling. Changing return type to boolean?

For the case where LoadCharacter is called using InvokeServer, the error message is reported to the client and the original callback thread is freed, so having no messages printed after LoadCharacter doesn’t necessarily mean that the yield is infinite.

However, I did see that there is a timing window where no errors are reported, that’s something that we will fix.

4 Likes

It’s been a while (a year+), but I remember having an issue with my game super skyward towers where a LoadCharacter() call, despite being pcall’d, would simply never return. After about 3 or 4 consecutive LoadCharacter() calls, the entire main thread that was running it would hang forever. No errors, warnings or anything. Code after the loadcharacter pcall would simply not execute.

1 Like

Thank you so much for looking into this!

1 Like

I would suspect that having something to do with the character appearance, even though it shouldn’t. When Roblox’s servers were down at one point and I was respawning, it would take forever to respawn before having a timeout error. Disabling character appearance fixed it for me.

1 Like

This documentation needs to be fixed.

The semantics around weak instance references are a LOT more unusual than this. The basic reasoning is that an Instance (lua) is not the same thing as an object (C++) in the datamodel.

When all references to the Lua instance are lost, the GC sweeps it, even if it hasn’t been destroyed. Its extremely unusual and requires some funny workarounds.

4 Likes

I would agree that this documentation needs rewording. After reading the line over a few times in good faith, I still have no clue what the author was intending to imply. Whatever the case, it’s deceptive, unclear, and should absolutely be changed.

For all practical purposes, I think the observed behavior here makes sense, if developers think of the Instance as merely a distributed interface, and not a direct representation of the underlying object itself. This interface needs to preserve the object (otherwise it can’t make changes), but it itself does not need to be maintained by anything but the user’s code.

I would also note that not all of the workarounds for weakly referencing Instances are too unwieldly. Sure, over the years, I’ve certainly seen a fair share of scripters get worked up about this issue, and write massive several hundred line solutions; but circumstantially, there’s a far simpler approach which has been good and feasible since the dawn of this problem: abstraction.

As an example, here’s a basic implementation of a method I designed a few years back (though there’s no shortage of elegant means for accomplishing the same feat through similar reasoning)

Example
local WeakTable = setmetatable({}, {__mode = 'v'})

local function CreateWeakInstanceReference(Object, AssociatedData)
	local SubstituteReference = Instance.new("Folder")
	SubstituteReference.Name = "ReferentialSubstitution"
	SubstituteReference.Parent = Object
	WeakTable[SubstituteReference] = AssociatedData
	Object:GetAttributeChangedSignal("ReferenceKeeper"):Connect(function()
		return AssociatedData
	end)
end

Here, I use a connection in order to associate some data (a complex object which isn’t an Instance) in a one-directional referential relationship with the underlying instance itself. This way, the instance preserves the object until it’s picked up by GC; meaning I can use this data as a value in a weak table, and that entry will vanish some time after the instance is collected. For the key, I create a child of that object, so that I can momentarily access the Instance I’m abstracting at any time, by grabbing the key’s .Parent (the abstracted instance itself cannot be used, as that would create a cyclic reference). Since only the values in this table are weak, the key won’t vanish until the entry is removed when the value is GCd; which, due to the relationship established by the connection, won’t happen until after the abstracted instance is collected.

1 Like

Is this still an issue? Have they fixed it?

1 Like

I wonder if this is related to a memory leak I found where the some of the memory taken up by spawned vehicles isn’t cleaned up after the vehicle being destroyed.

I can confirm that ROBLOX instances do not get garbage collected. I was able to find this when destroying vehicles:

I quoted an official statement in the roblox documentation that says they don’t get garbage collected when stored in weak tables. It’s very much intentional.

In the case of your example, it looks like it’s likely a fault of your code not properly cleaning up any references to those cars which is why the memory isn’t going down.

I set the car variable to nil after spawning it, all the rest of the references to the car model itself are within scripts inside the car, which get destroyed when the car is destroyed of course.

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.