sonoro1234 / LuaJIT-ImGui

LuaJIT ffi binding for imgui, backends and extension widgets
MIT License
225 stars 29 forks source link

cdata references returned from functions become dangling after garbage collection cycle #12

Closed THE-FYP closed 5 years ago

THE-FYP commented 5 years ago

This bug was introduced by my changes in #7, my apologies for that. When a cdata array of structs is indexed the array element is taken by reference (which is totally reasonable), not by value, but LuaJIT doesn't keep the parent object alive while the reference is alive, so when the temporary array is being collected, the reference become dangling. According to the FFI conversion semantics, struct is passed by a pointer when function takes a pointer, so instead of creating an array we just allocate a struct and pass it as is.

sonoro1234 commented 5 years ago

This bug was introduced by my changes in #7, my apologies for that.

This is my fault also.

When a cdata array of structs is indexed the array element is taken by reference

Yes, I have checked that empirically. But I have never experimented an issue with the dangling reference. Did you?

Even with

function GetMousePosO()
    local nonUDT_out = ffi.new("ImVec2[1]")
    ig.lib.igGetMousePos_nonUDT(nonUDT_out)
    local ret = nonUDT_out[0]
    nonUDT_out = nil
    collectgarbage();collectgarbage()
    return ret
end
THE-FYP commented 5 years ago

Yes, I did. It is easily reproducible this way:

do
local ffi = require 'ffi'
ffi.cdef 'typedef struct { int v; } st;'
local elem
do
   local arr = ffi.new('st[1]', {[0] = {v = 1337}})
   elem = arr[0]
end
print(elem.v)
assert(elem.v == 1337)
collectgarbage()
print(elem.v)
assert(elem.v == 1337)
end

My output:

1337
56
stdin:13: assertion failed!
stack traceback:
        [C]: in function 'assert'
        stdin:13: in main chunk
        [C]: at 0x01313330
sonoro1234 commented 5 years ago

Yes. Thankyou