Re: releasing callback on GC

["Robert G. Jakabosky" <bobby@...>]

You should be able to simplify the set_lua_callback() & push_lua_callback() 
functions so there is no weak table stored in the Registry.

/* Lua Stack: <userdata>, <function> */
void set_lua_callback(lua_State *L) {
  luaL_checktype(L, 2, LUA_TFUNCTION);
  /*create fenv table. */
  lua_newtable(L); /* stack: <userdata>, <function>, <table> */
  /* create new Lua thread */
  lua_ = lua_newthread(L); /* stack: <userdata>, <function>, <table>, <thread> 
*/
/* add a reference to the new thread in the userdata's fenv table,
   we don't need to keep the returned reference id, this is just for
  the GC to find a reference to the new thread.
*/
  luaL_ref(L, -2); /* stack: <userdata>, <function>, <table> */

  /* set fenv table as the userdata's environment. */
  lua_setfenv(L, 1); /* stack: <userdata>, <function> */

  /* push copies of callback <function> & <userdata> */
  lua_pushvalue(L, 2);
  lua_pushvalue(L, 1);
  /* L stack: <userdata>, <function>, <function>, <userdata> */

  /* now we move the callback <function> & <userdata> onto
     the thread's stack. */
  lua_xmove(L, lua_, 2);
  /* L stack: <userdata>, <function> */
  /* lua_ stack: <function>, <userdata> */
}

/* lua_ stack: <function>, <userdata> */
void push_lua_callback(bool push_self = true) {
  /* push another copy of callback function onto 'lua_' stack. */
  lua_pushvalue(lua_, 1);
  /* push another copy of userdata onto 'lua_' stack. */
  if(push_self) lua_pushvalue(lua_, 2);
}

The GC reference chain will be like this:
<userdata> -> <fenv table> -> <lua_ thread>
 -> (<callback function>, <userdata>)

As soon as there are no more references to <userdata> that whole chain will be 
released.

On Monday 03, Gaspard Bucher wrote:
> Hi Robert,
>
> Thanks a lot, I fixed my code by first creating the env table for the
> userdatum. Now I can create 200 threads that get killed on garbage
> collection and it works like a charm. I am storing "self" and "func" in a
> weak table that lives in the registry.
>
> Passing test case below. ;-)
>
> function should.create_many_threads_and_properly_gc()
>   -- warmup
>   make_threads() -- 200 threads that work and join
>
>   collectgarbage('collect')
>   local before = collectgarbage('count')
>   make_threads()
>
>   collectgarbage('collect')
>   local after = collectgarbage('count')
>   assert_equal(before, after)
> end
>
> Since the code is open source, if anyone stumbles on this message looking
> for a similar pattern, the implementation is here:
>
> http://bit.ly/hQ1ZbL
>
> Thanks to the list for the great support !
>
> PS: I feel some moonlight bliss
>
> On Mon, Jan 3, 2011 at 7:07 AM, Robert G. Jakabosky
>
> <bobby@sharedrealm.com>wrote:
> > On Sunday 02, Gaspard Bucher wrote:
> > > Hmmm.. I am nearly there. The function storage in the weak table works
> >
> > fine
> >
> > > except... I cannot get hold of the function through the userdata's
> > > environment. Stack is
> > >
> > > 1: userdata (self)
> > > 2: function
> > >
> > >     lua_getfenv(L, 1); // get environment for 'self'
> >
> > Did you create a table and call lua_setfenv() on the userdata object?
> >
> > >     // env.callback = func
> > >     lua_pushstring(L, "callback");
> > >     lua_pushvalue(L, 2); // push func on top
> > >     lua_settable(L, 3);
> > >
> > >     // env.thread = thread
> > >     lua_pushstring(L, "thread");
> > >     lua_ = lua_newthread(L);
> > >     lua_settable(L, 3);
> > >
> > >     lua_pop(L, 1); // remove env table
> >
> > That is the code for setting the callback & thread in the fenv table.
> >  Where
> > is the code that you use to get those values from the fenv later?
> >
> > Also you can pre-push the callback function onto the new lua_State
> > returned from lua_newthread(L) and then just pass the new lua_State to
> > your OS thread.
> >
> > > If I replace the calls with luaL_ref(L, LUA_REGISTRYINDEX), the
> > > function and/or thread are no longer collected.
> >
> > You can also call:
> > luaL_ref(L, fenv_idx); /* fenv_idx is the stack index of the fenv table.
> > */
> >
> > to allocate references in the userdata's fenv table.
> >
> > > The test code holds the created threads by inserting them in a table:
> > >
> > >
> > > function should.not_segfault()
> > >   local threads = {}
> > >   local functions = {}
> > >   collectgarbage('stop')
> > >   for i = 1,100 do
> > >     table.insert(threads, rk.Thread(function()
> > >       -- do nothing
> > >       local j = 0
> > >       while j < 10 do
> > >         print(i, j)
> > >         j = j + 1
> > >         worker:sleep(20)
> > >       end
> > >     end))
> > >     if i == 60 then
> > >       collectgarbage('collect')
> > >       print("collect ok.")
> > >     end
> > >   end
> > >
> > >   for _, thread in ipairs(threads) do
> > >     thread:join()
> > >   end
> > >
> > >   -- should not hang
> > >   assert_true(true)
> > > end
> > >
> > > Any idea what I am doing wrong ?
> > >
> > > Gaspard (6am here: going to bed)
> > >
> > > On Mon, Jan 3, 2011 at 3:40 AM, Gaspard Bucher <gaspard@teti.ch> wrote:
> > > > Storing the function in the environment table is one thing, but
> > > > getting it back from C is another...
> > > >
> > > > In order to get back to the function, I figured out this pattern:
> > > >
> > > > 0. On startup: create a weak table in the registry and store the
> >
> > location
> >
> > > > as a C global
> > > >
> > > > 1. push the weak table on top
> > > > lua_rawgeti(L, LUA_REGISTRYINDEX, g_weak_table_idx);
> > > >
> > > > 2. create reference
> > > > lua_pushvalue(L, -2);
> > > > int func_idx = luaL_ref(L, -1);
> > > >
> > > > .. later ..
> > > > 3. push weak table on top and get function
> > > > lua_rawgeti(L, LUA_REGISTRYINDEX, g_weak_table_idx);
> > > > lua_rawgeti(L, -1, func_idx);
> > > >
> > > > Does this look ok ?
> > > >
> > > > G.
> > > >
> > > > On Mon, Jan 3, 2011 at 3:01 AM, Gaspard Bucher <gaspard@teti.ch>
> >
> > wrote:
> > > >> Hi Peter,
> > > >>
> > > >> Thanks, this seems to be the correct way to handle my issue. I am
> > > >> looking at lua_getfenv and lua_setfenv and am not sure if this is
> > > >> how
> >
> > it
> >
> > > >> is meant to be used:
> > > >>
> > > >> -- callback function is at 1
> > > >> -- rk.Thread userdata is at 2
> > > >> lua_getfenv(L, 2);
> > > >> lua_pushvalue(L, 1);
> > > >> lua_pushstring(L, "callback");
> > > >> lua_settable(L, -3);
> > > >>
> > > >> G.
> > > >>
> > > >> On Mon, Jan 3, 2011 at 2:03 AM, Peter Odding
> >
> > <peter@peterodding.com>wrote:
> > > >>> Hi Gaspard,
> > > >>>
> > > >>>
> > > >>>  This is a difficult one: I have an OS thread that keeps hold of a
> > > >>>
> > > >>>> function in lua with luaL_ref(L, LUA_REGISTRYINDEX). But the OS
> >
> > thread
> >
> > > >>>> is created in Lua:
> > > >>>>
> > > >>>> thread = rk.Thread(function()
> > > >>>>   -- do this and that
> > > >>>> end
> > > >>>>
> > > >>>> After some time, the thread is garbage collected.
> > > >>>>
> > > >>>> How can I remove the function from the registry ? I cannot call
> > > >>>> luaL_unref in the rk.Thread destructor (segfault).
> > > >>>
> > > >>> I'm not sure I have the full picture here, but why not give your
> >
> > thread
> >
> > > >>> userdata an environment table and then keep hold of the function in
> > > >>> that environment table instead of in the global registry? Once your
> > > >>> thread is garbage collected the environment table should be
> > > >>> destroyed and any references in that table should automatically
> > > >>> seize to exist.
> > > >>>
> > > >>>  - Peter Odding
> >
> > --
> > Robert G. Jakabosky



-- 
Robert G. Jakabosky