Garbage collection of userdata inside userdata

[Stefano Cossu]

Hello,

I have a Lua C module defining two different userdata types, one is
called a Dataset and the other a TermSet. Both can be used and
garbage-collected independently. A Dataset object has a TermSet as a
"gr" attribute. I am setting this term set as a uservalue for the dataset.

The problem is that if I access the term set from the dataset, it gets
garbage collected when it falls out of scope, and then, when the dataset
is garbage collected too, it calls a free function that attempts to free
the term set that was already freed, resulting in a double-free error.

This is the function used for creating the object:

// Called by dataset factories
int dataset_to_udata (lua_State *L, VOLK_Store *store)
{
VOLK_Dataset **dsp = lua_newuserdatauv (L, sizeof (*dsp), 2);
luaL_getmetatable (L, "volksdata.Dataset");
lua_setmetatable (L, -2);
VOLK_Dataset *ds = VOLK_dataset_new (store);
LUA_NLCHECK (ds, "Error creating dataset.");
*dsp = ds;

// Set store uservalue.
lua_pushvalue(L, 1);
lua_setiuservalue (L, -2, 1);

// Set gr uservalue.
VOLK_TermSet **ts_p = lua_newuserdata (L, sizeof (*ts_p));
LUA_NLCHECK (ts_p, "Error allocating memory for term set.");
luaL_getmetatable (L, "volksdata.TermSet");
lua_setmetatable (L, -2);
lua_setiuservalue (L, -2, 2);

return 1;
}

And the access function:

static int
get_graphs (lua_State *L)
{
(void) check_dataset (L, 1);
lua_getiuservalue (L, 1, 2);

return 1;
}

I thought that setting the uservalue would prevent the term set from
being garbage collected when it falls out of scope, but I guess that it
still gets garbage collected when the dataset is destroyed.

Which approach shall I use? I guess I'd want to prevent the independent
term set from ever being garbage collected. I can't use a light userdata
pointer because the access function must return a full usable Lua
object. I also shouldn't duplicate the term set because it's expensive
and the two copies could fall out of sync, which is undesirable.

Thanks for any suggestions.
s

[Sean Conner]

It was thus said that the Great 'Stefano Cossu' via lua-l once stated:
> Hello,

Hello.

> Which approach shall I use? I guess I'd want to prevent the independent
> term set from ever being garbage collected. I can't use a light userdata
> pointer because the access function must return a full usable Lua
> object. I also shouldn't duplicate the term set because it's expensive
> and the two copies could fall out of sync, which is undesirable.

You might want to add a reference to the TermSet to the DataSet using
LUA_REGISTRYINDEX with the luaL_ref() and luaL_unref() functions. Using
luaL_ref() will anchor the TermSet until luaL_unref() is called which will
then allow the TermSet to be GCed.

-spc

[TopchetoEU]

If your objects are GC-managed, conceptually, you should *never* free them
manually. Instead, what I'd do is have the term set in one uservalue slot in
the dataset userdata. This way, dataset will keep its term set alive until it
itself falls out of scope *by design*.

Furthermore, do all deallocation stuff only in the `__gc` metamethod of the
uservalues, and *never* deallocate one GC'd object's data from aonther's
`__gc` metamethod.

P.S.: Since lua 5.3-ish (afaik), userdata can have multiple uservalue slots,
use that to your advantage.

[Stefano]

Thanks, Sean. What you suggested worked. Initially I was hesitant to try it because I thought that even if I tie the term set to the data set lifespan, it would still be double-freed. 

But the tests now pass and Valgrind doesn't raise any invalid read/free warnings. 

TopchetoEU: the only place I am freeing the pointers is in the __gc methods related to the Lua objects containing the C data. The complication with this particular structure is that the data set is meant to free the contained term set in its C free function, but also, when I access the term set from the data set in Lua, a new Lua term set object is created, that points to the structure owned by the data set. Since a term set can also be an independently created object, I must define a __gc method for that too.