Process abort when setting a record field with a callable

[jdesgats]

Note: I'm a total noob on GLib so I'm not sure if it is a bug or a misuse and even less if my fix is the most cleaver one !

I was playing with DBus APIs when I experienced process aborts while setting objects vtables. Here is a sample to reproduce the bug:

lgi = require 'lgi'
Gio = lgi.require 'Gio'

vtable = Gio.DBusInterfaceVTable()
vtable.get_property = function() end

This code causes the following:

$ lua crash.lua 

** (process:19235): CRITICAL **: g_arg_info_get_scope: assertion `info != NULL' failed
**
Lgi:ERROR:marshal.c:758:marshal_2c_callable: assertion failed: (scope == GI_SCOPE_TYPE_ASYNC)
[1]    19235 abort (core dumped)  lua crash.lua

The provided fix seem to do the work and does not leak memory.

[pavouk]

Thanks a lot for good report, testcase and an effort to fix it. Unfortunately I don't think that the patch is correct; setting scope to CALL_TYPE will cause lgi_marshal_2c() call to actually produce one stray value on the stack (assuming that caller will clean it after the function call, causing release of the resources bound to the closure). But lgi_marshal_field() does not count with the possibility that there is some leftover on the stack from the assignment, the value is 'forgotten' on the lua stack and sometime later collected by GC (together with closure trampoline). That means that the code will most probably crash when callback is invoked after some GC cycle.

The real problem here is that we have to find some place where to anchor the closure resources - in other words, assigning Lua function to callback actually produces some value which must be kept somewhere as long as the callback exists and can be invoked - in this case ideally to the DBusInterfaceVTable instance. But it is not straightforward, as this struct does not have any spare field to be used for that.

The basics for this work already exists in wip/subclass branch; there is new API local guard, addr = core.marshal.callback(callbackinfo, func) which creates closure and guard keeping the resources associated with the closure alive and it is up to caller to store it somewhere safe while the callback can be called. In this case it would be used something like this:

local lgi = require 'lgi'
local Gio = lgi.require 'Gio'
local core = require 'lgi.core'

local vtable = Gio.DBusInterfaceVTable()
local function get_property() end
local get_prop_guard, get_prop_addr = core.marshal.callback(Gio.DBusInterfaceGetPropertyFunc, get_property())
vtable.get_property = get_prop_addr
-- And now we need to store get_prop_guard somewhere and keep it alive as long as vtable is alive.

Frankly, the really fix will have to probably wait for wip/subclass merge (which is going to happen real soon now(r)), and after that, I should cover the ugliness sketched above into some lgi/override/Gio-DBus.lua coat.

[jdesgats]

Thanks for this detailed and instructive feedback. Like I said, I don't have a strong knowledge of GObject system so my fix was just a guess. And yes, my fix does not work anymore after a forced GC cycle...

However, about override, this issue is not specific to DBus but is generic to any struct type having callable fields, no ? So does it mean that an override will be necessary each time it happen ?

Thanks anyway for making Lgi, it opens a lot of interesting possibilities for Lua !

[v1993]

I'm unable to reproduce this, probably this PR can be closed?