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

CallReference from C++ code leaves function reference behind #2967

Open
niekschoemaker opened this issue Nov 28, 2024 · 0 comments
Open

CallReference from C++ code leaves function reference behind #2967

niekschoemaker opened this issue Nov 28, 2024 · 0 comments
Labels
bug ScRT: Lua Issues/PRs related to the Lua scripting runtime triage Needs a preliminary assessment to determine the urgency and required action

Comments

@niekschoemaker
Copy link
Contributor

What happened?

When a reference get's called from C++ code without any script environment (ie. StateBag change handlers) and they have an async return value (calling Citizen.Wait from inside said handler) the reference get's leaked/left behind.

The specific function which is the probable cause of this is this piece of code scheduler.lua#L532

Expected result

Reference should be deleted after use

Reproduction steps

  1. Create state bag change handler with a wait inside
  2. Invoke the change handler by changing the state bag value (doesn't matter if it's within same resource or not)
  3. Reference get's left behind
--Reproduction code

-- Get MakeFunctionReference local from scheduler
local function getMakeFunctionReference()
    local i = 1
    while true do
        local n, v = debug.getupvalue(Citizen.GetFunctionReference, i)
        if not n then
            break
        end
        
        if n == "MakeFunctionReference" then
            return v
        end
        
        i = i + 1
    end
end

local funcRefs
-- Get funcRefs local from scheduler using MakeFunctionReference upvalues
local function getFuncRefsTable()
    if funcRefs == nil then
        local func = getMakeFunctionReference()
        if not func then
            funcRefs = false
            return nil
        end
        local i = 1
        while true do
            local n, v = debug.getupvalue(func, i)
            if not n then
                break
            end
            
            if n == "funcRefs" then
                funcRefs = v
                return v
            end
            i = i + 1
        end
        funcRefs = false
        return nil
    end
    
    return funcRefs
end

AddStateBagChangeHandler(nil, nil, function(bagName, key, value, reserved, replicated)
    Citizen.Wait(0)
end)

Citizen.CreateThread(function()
    Citizen.Wait(1000)
    local funcRefs = getFuncRefsTable()
    
    -- Add metatable to print whenever new ref get's created
    setmetatable(funcRefs, {
        __newindex = function(self, k, v)
            local stackTrace = FormatStackTrace()
            print(("%s: %s\n%s\n%s"):format(k, v, stackTrace, debug.traceback()))
            rawset(self, k, v)
        end,
    })
    
    local i = 0
    while i < 10 do
        collectgarbage()
        local count = 0
        for k,v in pairs(funcRefs) do
            count += 1
        end
        print("refCount", count)
        GlobalState.test = math.random(-math.maxinteger, math.maxinteger)
        collectgarbage()
        local count = 0
        for k,v in pairs(funcRefs) do
            count += 1
        end
        -- ref count here is one higher than above
        print("refCount", count)
        i += 1
        Citizen.Wait(100)
    end
end)

Importancy

There's a workaround

Area(s)

ScRT: Lua

Specific version(s)

Server 11535 Linux

Additional information

The main issue here is that the ref counter never get's incremented, and never get's decremented, so the DeleteRefRoutine function never get's invoked.

With simple use cases where the handler isn't asynchronous the code at ResourceCallbackComponent.h#L83 handles this correctly.

The expected behavior when using async handlers is normally:

  1. invoked resource packs the async return callback, function reference get's created
  2. original resource unpacks the callback reference, calls DuplicateFunctionReference
  3. GC runs, DeleteFunctionReference get's called, and reference is cleaned up.

Since this behavior is dependant on the script runtime only the first step is ran when an async handler is used anywhere where it doesn't end up in a script runtime (like said above, statebag change handlers mostly, and one other case is presentCard in the deferrals, not sure if there's more cases)

The impact with this isn't too large, but I noticed it since I was checking out some metrics on ref counts using my sketchy code above to try to diagnose some weird memory growth over time, and realized it was caused by a logger I had on state bags with a wait in it.

When there's a lot of state bag updates and the handler runs on every state bag this can add up to quite a large amount of unecessary memory usage.

@niekschoemaker niekschoemaker added bug triage Needs a preliminary assessment to determine the urgency and required action labels Nov 28, 2024
@github-actions github-actions bot added the ScRT: Lua Issues/PRs related to the Lua scripting runtime label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ScRT: Lua Issues/PRs related to the Lua scripting runtime triage Needs a preliminary assessment to determine the urgency and required action
Projects
None yet
Development

No branches or pull requests

1 participant