Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_queuedResolve, _queuedReject, and _queuedFinally can unnecessarily prevent garbage collection #39

Closed
Corecii opened this issue Aug 22, 2020 · 0 comments

Comments

@Corecii
Copy link

Corecii commented Aug 22, 2020

_queuedResolve = {},
_queuedReject = {},
_queuedFinally = {},

If these are not cleared when a promise completes, this can lead to unnecessary prevention of GC. An :andThen or :finally can include callbacks with upvalue references. Since promises keep strong references to these callbacks, the callbacks and their upvalue references will never be collected.

Here's a realistic example:

local gameReadyPromise = getGameReadyPromise() -- this promise exists forever and completes when all modules have loaded
-- ...
game.Players.PlayerAdded:Connect(function(player)
    gameReadyPromise:andThen(function()
        print(player.Name, "has joined the game!")
        -- load the player into the game
    end)
end)

If any players join the game before gameReadyPromise completes then their player objects can never be garbage collected ultimately due to references originating in _queuedResolve.

Here's a simple test:

local function testGc(callback)
	local gcTester = {}
	local weakRef = setmetatable({gcTester}, {__mode = "v"})
	callback(gcTester)
	gcTester = nil
	wait(10)
	return #weakRef == 0 and "GCed" or "Not GCed"
end

local persistentPromise = Promise.delay(0)

local didGetGced = testGc(function(upvalueGcTester)
	persistentPromise:andThen(function()
		local thisUsesUpvalue = upvalueGcTester
	end)
	persistentPromise:await()
	-- the above callback will *never* be called again, so ideally the callback
	-- and upvalueGcTester would be garbage collected, but they won't be:
end)

print("Did upvalueGcTester get collected?:", didGetGced)

This can be solved by clearing out or replacing _queuedResolve, _queuedReject, and _queuedFinally when the promise completes (such as in _finalize).

@evaera evaera closed this as completed in e7a01c7 Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant