Does this kill old connections and prevent memory leaks?

Help and Feedback Scripting Support
, ,
1 
local Connections do
	local Connected = {}
	local _Internal = {}

	function Connected:Add(UserId: number, Connection: RBXScriptConnection)
		if not _Internal[UserId] then
			_Internal[UserId] = {}
		end

		table.insert(_Internal[UserId], Connection)
	end

	function Connected:Disconnect(UserId: number)
		local UserConnections = _Internal[UserId]

		if UserConnections then
			for _, Connection in ipairs(UserConnections) do
				if Connection.Connected then
					Connection:Disconnect()
				end
			end
			_Internal[UserId] = nil
		end
	end

	Connections = table.freeze(Connected)
end

	Connections:Add(self.Player.UserId, NewValue:GetPropertyChangedSignal("Value"):Connect(function()
		Changed:Fire(NewValue.Value)
	end))

Players.PlayerRemoving:Connect(function(Player)
	Connections:Disconnect(Player.UserId)
end)

game:BindToClose(function()
	for _, Player in Players:GetPlayers() do
		Connections:Disconnect(Player.UserId)
	end
end)

Also is the last line needed or PlayerRemoving is just as a good

2

Yes, your setup correctly kills old connections per user and prevents memory leaks.

1 Like
3

Alright I was having questions about if I was doing things correctly as the console didn’t give me promising results like I saw others explain, anywho is the bindtoclose needed?

4

Looks good to me and well thought-out.

1 Like
5

Alright thank you for your feedback

1 Like
6

My two cents:

  1. You don’t need :BindToClose(), it fires when the server is going to shut down and literally everything in it is going to be destroyed anyway

  2. Why overcomplicate the problem with metatables?

    I’d just recommend storing whatever player-dependent connections you have in a table

    local connections = {}

Players.PlayerAdded:Connect(function(player)
	connections[player.UserId] = {}
end)

    adding them however you prefer

    table.insert(connections[player.UserId], NewValue.Changed:Connect(function()
	-- NewValue changed!

	-- By the way, on ValueBase instances, .Changed only fires when
	-- the Value property changes. See the second paragraph of
	-- https://create.roblox.com/docs/reference/engine/classes/Object#Changed

	-- (That means you don't need GetPropertyChangedSignal)
end))

    and cleaning them up afterwards

    Players.PlayerRemoving:Connect(function(player)
	for _, connection in connections[player.UserId] do
		if connection.Connected then
			connection:Disconnect()
		end
	end
end)

    If you need the table of connections shared across scripts, just define connections in a module and require it from every script you need them in. (That can mean a content of just return {})

I’d also say that you generally shouldn’t need to do this, but there are of course myriad legitimate use cases for a ton of connections coupled with the player outside of the PlayerObject.

I’m just thinking, if you’re using *Value objects for each player, perhaps in ServerStorage, and binding to .Changed for example, why not destroy them? That’s the easiest way to clean up connections.

also obligatory sleitnick utility link (this module handles everything you might be dealing with now or in the future regarding cleanup, not just connections)

7

I have been messing with Maid for some time.

1 Like