Correct garbage collecting the unhook ticket.
authorRui Guo <firemeteor.guo@gmail.com>
Tue, 16 Jun 2009 15:08:44 +0000 (16 23:08 +0800)
committerRui Guo <firemeteor.guo@gmail.com>
Tue, 16 Jun 2009 15:08:44 +0000 (16 23:08 +0800)
1. Also register unhook ticket for func registered with name.
2. We need two passes of GC collection after last commit.

src/lua.c

index 52c446b..35b5960 100644 (file)
--- a/src/lua.c
+++ b/src/lua.c
@@ -783,6 +783,9 @@ screen_input(lua_State *L)
 
   prompt = (char *)luaL_checkstring(L, 1);
   lh = LuaCheckHandler(L, 2, 1);
+  if (!lh)
+    luaL_error(L, "Out of Memory");
+
   sidata = (struct sinput_data *)malloc(sizeof(struct sinput_data));
   if (!sidata)
     luaL_error(L, "Out of Memory");
@@ -840,7 +843,6 @@ prepare_weak_table(lua_State *L, const char *name, const char *mode)
   lua_pop(L, 1);
 }
 
-/* FIXME: Think about this: will it affect the registered handlers?*/
 static lua_State *L;
 int LuaInit(void)
 {
@@ -868,7 +870,7 @@ int LuaInit(void)
    *
    * To make this process happens faster, GC is forced at the end of each
    * source. 
-   * TODO: What if some events are triggered with in the sourcing
+   * TODO: What if some events are triggered within the sourcing
    * procedure?
    * */
   prepare_weak_table(L, "_funcreg", "v");
@@ -921,6 +923,7 @@ LuaCallProcess(const char *name, struct fn_def defs[])
   return 1;
 }
 
+/* FIXME: Think about this: will it affect the registered handlers?*/
 int LuaSource(const char *file, int async)
 {
   if (!L)
@@ -939,7 +942,21 @@ int LuaSource(const char *file, int async)
           return 0;
        }
       else
-        lua_gc(L, LUA_GCCOLLECT, 0);
+        {
+          /* It seems that I need two GC passes to really collect the unhook
+           * ticket, after changing the reference to ticket in the
+           * 'funcunhook' table from weak to strong. This should not be
+           * harmful, but how can I make sure that two passes is enough?
+           *
+           * Possible reason:
+           * This seems reasonable, since the first pass will collect the func
+           * itself and make the ticket garbage, and the second pass will
+           * collect the ticket itself.
+           *
+           * TODO: check this out. maybe ask some lua gurus. */
+          lua_gc(L, LUA_GCCOLLECT, 0);
+          lua_gc(L, LUA_GCCOLLECT, 0);
+        }
       return 1;
     }
   return 0;
@@ -1162,6 +1179,20 @@ int LuaForeWindowChanged(void)
   return LuaCallProcess("fore_changed", params);
 }
 
+/*FIXME: what if a func is registered twice or more? */
+void
+LuaRegAutoUnHook(lua_State *L, lua_handler lh, int ticket)
+{
+  int sc, funcunhook;
+  luaL_getmetatable(L, "screen");
+  sc = lua_gettop(L);
+  funcunhook = LuaPushHTable(L, sc, "_funcunhook");
+  LuaPushHandler(L, lh);
+  lua_pushvalue(L, ticket);
+  lua_rawset(L, funcunhook);
+  lua_pop(L, 2);
+}
+
 #define SEVNAME_MAX 30
 static int
 LuaRegEvent(lua_State *L)
@@ -1169,7 +1200,7 @@ LuaRegEvent(lua_State *L)
   /* signature: hook(obj, event, handler, priv);
    *        or: hook(event, handler, priv)
    *   returns: A ticket for later unregister. */
-  int idx = 1, func, argc = lua_gettop(L);
+  int idx = 1, argc = lua_gettop(L);
   unsigned int priv = 31; /* Default privilege */
   lua_handler lh;
 
@@ -1199,7 +1230,6 @@ LuaRegEvent(lua_State *L)
   snprintf(evbuf, SEVNAME_MAX, "%s_%s", objname, event);
 
   /* Check and get the handler */
-  func = idx;
   lh = LuaCheckHandler(L, idx++, 1);
   if (!lh)
     luaL_error(L, "Out of memory");
@@ -1225,17 +1255,7 @@ LuaRegEvent(lua_State *L)
       push_callback(L, &l);
       ticket = lua_gettop(L);
 
-      if (lh->type == LUA_HANDLER_TYPE_F)
-        {
-          int sc, funcreg;
-          luaL_getmetatable(L, "screen");
-          sc = lua_gettop(L);
-          funcreg = LuaPushHTable(L, sc, "_funcunhook");
-          lua_pushvalue(L, func);
-          lua_pushvalue(L, ticket);
-          lua_rawset(L, funcreg);
-          lua_pop(L, 2);
-        }
+      LuaRegAutoUnHook(L, lh, ticket);
     }
   else
     return luaL_error(L, "Invalid event specified: %s for object %s", event, objname);