PSA: Connections can memory leak Instances!

Interesting, thanks!

I recommend putting this in #tutorials

2 Likes

I’ve been using this to disconnect one-time use connections;

local con
con = something.something:connect(function() 
print("works")
con:disconnect()
end)

That way once the connection has been used for its purpose, it should no longer exist. I can’t see any immediate issues with it, anything I’ve overlooked?

4 Likes

Wouldn’t the :Wait() method be better for single usage events?

local passedArgs = something.something:Wait()
print("works")
3 Likes

That’s possible, but if “something” can be destroyed, that :Wait() will hang forever. And if you want to do code after it, you’ll have to wrap it in its own thread anyway, hence an event connection would be cleaner.

10 Likes

Thanks for the reply! So just to reiterate, using :disconnect() on all connections you make (once you’re done with them) cleans up this issue?

1 Like

Doesn’t ROBLOX’s default Health script cause a nasty memory leak every time a player dies?

-- Gradually regenerates the Humanoid's Health over time.

local REGEN_RATE = 1/100 -- Regenerate this fraction of MaxHealth per second.
local REGEN_STEP = 1 -- Wait this long between each regeneration step.

--------------------------------------------------------------------------------

local Character = script.Parent
local Humanoid = Character:WaitForChild'Humanoid'

--------------------------------------------------------------------------------

while true do
	while Humanoid.Health < Humanoid.MaxHealth do
		local dt = wait(REGEN_STEP)
		local dh = dt*REGEN_RATE*Humanoid.MaxHealth
		Humanoid.Health = math.min(Humanoid.Health + dh, Humanoid.MaxHealth)
	end
	Humanoid.HealthChanged:Wait()
end

Since characters never get :destroy() called on them, they just get parented to nil. This is also the case for Player instances.

7 Likes

This has been my suspicion for quite a while. It’s nice to know that the closure is aware of usage underneath it. I’d highly recommend disconnecting all events you don’t intend to be permanent.

General notes:

  • :Destroy() is recursive so you only have to call it on the parent hierarchy
  • If your client scripts don’t persistent when the player dies, you may be fine. This isn’t a great assumption, but Roblox definitely disconnects scripts on death. Module scripts are an exemption to this, and don’t disconnect (for good reason)

I stole this pattern from Anaminus, although my implementation does some nicer things when nested Maids and on-cleanup calls.

Lua has weak tables, but as we’ve discussed before, these don’t work out too nicely for GC.

No, these scripts in the character and definitely removed / destroyed in some manner.

while true do
	wait(0.5)
	print("Still running script", script:GetFullName())
end

Stops execution after death. This implies that signals are disconnected as well.

For players themselves, I’m not sure. I always disconnect events on players. I think scripts inside of them are also destroyed.

10 Likes

It seems scripts specifically within the character are disabled, but events connected to the character from elsewhere are not. So if you have a HealthChanged function (for example) connected to players OnCharacterAdded from the server, this connection does not disappear just because the character dies. The garbagecollection for this event then follows the rules posted in the OP. I’d imagine this is the same case for players.

2 Likes

This post right here will fix a lot of lag issues in my game. Great post! :slight_smile:

(You might want to consider moving this to the public tutorial section. This is seriously useful information.)

3 Likes

Could the core issue be worked around with a backport of Lua 5.2’s ephemeron weak tables?

1 Like

Yes, I believe it could… you could use an ephemeron table to hold references to Lua functions in connection lists. However, it suspect that it would also make the Lua bridge much more complicated:

  • The Lua bridge would have to be “aware” of something being passed over it is being used for, since the closure passed to connect would have to be associated with object whose connection list it is being stored in when being put in the ephemeron. Currently it’s just a flat system opaquely passing values across.

  • You would probably have to hold a strong reference to the Lua-side “Ref” object associated with the instance while that instance is in the object hierarchy and it has any connections to Lua functions (Note: The connection interface is uniform between C++ and Lua, the engine can connect events to C++ functions too) so that the closure doesn’t get GCed while the object still exists on the C++ side (currently unless you get a reference to a given instance, that instance doesn’t exist on the Lua side at all—even though it may have connections to Lua functions which don’t reference it)

  • There is probably additional complexity involved in getting RBXScriptSignal::Wait to function correctly in all circumstances with this ephemeron implementation.

TL;DR: It would definitely be a non-trivial project even if the backporting part were free.

1 Like

connect() is deprecated, you’re spreading false info!111!1!!

Jk, good guide and definitely something I never thought about.

5 Likes

I’m an old fogey… so uppercase Workspace and lowercase connect it is.

20 Likes

I’ll probably keep using lowercase connect myself until they start throwing warnings for it in intellisense. It’d be too weird to switch otherwise.

4 Likes

I use :connect because that way my eyes will focus on the PascalCased event name. Of course, we’re going off topic here, but…

5 Likes

I prefer :connect() in my code

2 Likes

The only issue with using :connect() versus :Connect() is being forced later to change it all. For now they are exactly the same, but if some kind of major issue is discovered/new method is applied, it will be applied to Connect and not to connect. Using the lowercase version is like sticking to an old OS; it still will work, but over time it can get more and more out of date, potentially exposing flaws or lacking features that are actually required. In the end it is a matter of change now with little effort, or change later with a ton of effort.

2 Likes

Both Connect and connect point to the exact same function in memory.

4 Likes

That is exactly what I said.

2 Likes

No it won’t. If it applies to Connect, it’ll automatically apply to connect. They point to the same function in memory. Your statement would be true if they both pointed to different, identical functions.

2 Likes