Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Commit

Permalink
Experimental warning to help find connections that are not disconnected
Browse files Browse the repository at this point in the history
This commit introduces a new configuration option which aims to help Roact users more easily find components which are not properly disconnecting connections when unmounted and are potentially causing memory leaks because of this reason.

A full traversal of the component is performed to find all currently active connections but some keys are excluded to reduce false positives.

If a connection is referenced in a component but it's closure does not close over self as an upvalue then it is not a concern that the component still has a reference after being unmounted.
This commonly occurs through references to component context or the store. Ideally we would be able to inspect the closure associated with each active connection and warn only if the closure uses self as an upvalue.

Internal Roact connections are also ignored for this check, so that connections within EventManager do not cause this warning to be emitted.
  • Loading branch information
DarraghGriffin committed Nov 23, 2020
1 parent 380c3d6 commit 5d0edbf
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 0 deletions.
31 changes: 31 additions & 0 deletions src/Component.lua
Expand Up @@ -2,6 +2,7 @@ local assign = require(script.Parent.assign)
local ComponentLifecyclePhase = require(script.Parent.ComponentLifecyclePhase)
local Type = require(script.Parent.Type)
local Symbol = require(script.Parent.Symbol)
local getActiveConnections = require(script.Parent.getActiveConnections)
local invalidSetStateMessages = require(script.Parent.invalidSetStateMessages)
local internalAssert = require(script.Parent.internalAssert)

Expand Down Expand Up @@ -367,6 +368,36 @@ function Component:__unmount()
for _, childNode in pairs(virtualNode.children) do
reconciler.unmountVirtualNode(childNode)
end

if config.experimentalActiveConnectionsWarning then
--[[
A full traversal of the component is performed but some keys are excluded to reduce false positives.
If a connection is referenced in a component but it's closure does not close over
self as an upvalue then it is not a concern that the component still has a reference
after being unmounted. This commonly occurs through references to component context or the store.
Ideally we would be able to inspect the closure associated with each active connection
and warn only if the closure uses self as an upvalue.
Internal Roact connections are also ignored for this check, so that connections within EventManager
do not cause this warning to be emitted.
--]]
local ignoreKeys = {
InternalData,
"_context",
"store",
}
local activeConnections = getActiveConnections(self, ignoreKeys, self.__componentName)
for name, _ in pairs(activeConnections) do
warn("Roact experimental active connection warning:\n" ..
self.__componentName.. " did not clear connection " .. name .. "\n" ..
"Note: This warning is experimental and may reflect an issue with the implementation of your component\n" ..
"If the closure referenced closes self as an upvalue then this connection needs to be disconnected " ..
"so that this component will be garbage collected and this won't cause a memory leak."
)
end
end
end
--[[
Expand Down
3 changes: 3 additions & 0 deletions src/Config.lua
Expand Up @@ -22,6 +22,9 @@ local defaultConfig = {
["elementTracing"] = false,
-- Enables validation of component props in stateful components.
["propValidation"] = false,

-- Enables an experimental warning which can help to track down connections not being disconnected
["experimentalActiveConnectionsWarning"] = false,
}

-- Build a list of valid configuration values up for debug messages.
Expand Down
38 changes: 38 additions & 0 deletions src/getActiveConnections.lua
@@ -0,0 +1,38 @@
local function getActiveConnections(obj, ignoreKeys, name)
local seen = {}
local results = {}

local function shouldIgnoreKey(key)
for _, k in pairs(ignoreKeys) do
if k == key then
return true
end
end
return false
end

local function collectActiveConnections(child, namespace)
if seen[child] then
-- Break out of cyclical tables
return
end
seen[child] = true

for k, v in pairs(child) do
if not shouldIgnoreKey(k) then
local kStr = tostring(k)
local fullNameKey = namespace == "" and kStr or namespace.. "." ..kStr
if typeof(v) == "table" then
collectActiveConnections(v, fullNameKey)
elseif typeof(v) == "RBXScriptConnection" and v.Connected then
results[fullNameKey] = v
end
end
end
end

collectActiveConnections(obj, name)
return results
end

return getActiveConnections

0 comments on commit 5d0edbf

Please sign in to comment.