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

Bug Reports Engine Bugs
#43

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
#44

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
#45

Thank you so much for looking into this!

1 Like
#46

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
#47

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
#48

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
#49

Is this still an issue? Have they fixed it?

1 Like
#50

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.

#51

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

#52

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.

#53

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.

#54

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

#55
#56