Fix refcount logic & duplicate unhook checking.
authorRui Guo <firemeteor.guo@gmail.com>
Fri, 12 Jun 2009 14:22:58 +0000 (12 22:22 +0800)
committerRui Guo <firemeteor.guo@gmail.com>
Fri, 12 Jun 2009 14:22:58 +0000 (12 22:22 +0800)
1. LuaFuncKey() now increase the ref count when asked to, but never decrease
   the ref count. It may return LUA_UNREF if an entry is not found but asked
   not to add it.
2. LuaCheckHandler never decreases ref count and record function name only
   for mapped entry.
3. Require to provide the function name on unhook, if the function could not
   be resolved to key, do not proceed.

src/lua.c
src/scripts/cmdcallback.lua

index 9357a9e..a5a53a0 100644 (file)
--- a/src/lua.c
+++ b/src/lua.c
@@ -894,8 +894,40 @@ LuaPushHandler(lua_State *L, lua_handler lh)
   lua_pop(L, 1);
 }
 
+void
+LuaHRef(lua_State *L, int key, int reg)
+{
+  int funckey, keyfunc, cnt, sc, idx;
+  luaL_getmetatable(L, "screen");
+  sc = lua_gettop(L);
+  funckey = LuaPushHTable(L, sc, "_funckey");
+  keyfunc = LuaPushHTable(L, sc, "_keyfunc");
+
+  lua_rawgeti(L, keyfunc, key);
+  idx = lua_gettop(L);
+
+  lua_rawgeti(L, funckey, key);
+  cnt = lua_tointeger(L, -1);/*cnt = htable[key]*/
+  if (!reg && cnt <= 1)
+    {
+      /*release the last reference*/
+      luaL_unref(L, funckey, key);
+      lua_pushvalue(L, idx);
+      lua_pushnil(L);
+      lua_settable(L, funckey); /*htable[func]=key*/
+    }
+  else
+    {
+      lua_pushinteger(L, key);
+      lua_pushinteger(L, reg? cnt + 1 : cnt - 1);
+      lua_settable(L, funckey); /*htable[key] = cnt + 1*/
+    }
+  lua_pop(L, 5);
+}
+
+/* Ref when asked to. Never unref*/
 int
-LuaFuncKey(lua_State *L, int idx)
+LuaFuncKey(lua_State *L, int idx, int ref)
 {
   int key, funckey, sc;
   luaL_getmetatable(L, "screen");
@@ -907,6 +939,8 @@ LuaFuncKey(lua_State *L, int idx)
   if (lua_isnil(L, -1))
     {
       /* Not found, alloc a new key */
+      if (!ref)
+        return LUA_NOREF;
 
       /*funckey[key] = 1*/
       lua_pushinteger(L, 1);
@@ -926,44 +960,17 @@ LuaFuncKey(lua_State *L, int idx)
       lua_pop(L, 1);
     }
   else
-    key = lua_tointeger(L, -1);/*key = funckey[func]*/
+    {
+      key = lua_tointeger(L, -1);/*key = funckey[func]*/
+      if (ref)
+        LuaHRef(L, key, 1);
+    }
 
   lua_pop(L, 3);
   return key;
 }
 
 void
-LuaHRef(lua_State *L, int key, int reg)
-{
-  int funckey, keyfunc, cnt, sc, idx;
-  luaL_getmetatable(L, "screen");
-  sc = lua_gettop(L);
-  funckey = LuaPushHTable(L, sc, "_funckey");
-  keyfunc = LuaPushHTable(L, sc, "_keyfunc");
-
-  lua_rawgeti(L, keyfunc, key);
-  idx = lua_gettop(L);
-
-  lua_rawgeti(L, funckey, key);
-  cnt = lua_tointeger(L, -1);/*cnt = htable[key]*/
-  if (!reg && cnt <= 1)
-    {
-      /*release the last reference*/
-      luaL_unref(L, funckey, key);
-      lua_pushvalue(L, idx);
-      lua_pushnil(L);
-      lua_settable(L, funckey); /*htable[func]=key*/
-    }
-  else
-    {
-      lua_pushinteger(L, key);
-      lua_pushinteger(L, reg? cnt + 1 : cnt - 1);
-      lua_settable(L, funckey); /*htable[key] = cnt + 1*/
-    }
-  lua_pop(L, 5);
-}
-
-void
 LuaHSetName(lua_State *L, int key, int name)
 {
   int keyfunc, sc;
@@ -996,7 +1003,7 @@ LuaHGetName(lua_State *L, lua_handler key)
 }
 
 lua_handler
-LuaCheckHandler(lua_State *L, int idx, int reg)
+LuaCheckHandler(lua_State *L, int idx, int ref)
 {
   int name = 0, key;
   if (lua_isstring(L, idx))
@@ -1012,9 +1019,8 @@ LuaCheckHandler(lua_State *L, int idx, int reg)
   else if (!lua_isfunction(L, idx))
     luaL_error(L, "Handler should be a function or the name of function");
 
-  key = LuaFuncKey(L, idx );
-  LuaHRef(L, key, reg);
-  if (name)
+  key = LuaFuncKey(L, idx, ref);
+  if (name && key != LUA_NOREF)
     {
       LuaHSetName(L, key, name);
       lua_pop(L, 1);
@@ -1147,10 +1153,11 @@ LuaRegEvent(lua_State *L)
 static int
 LuaUnRegEvent(lua_State *L)
 {
-  /* signature: unhook([obj], ticket)
+  /* signature: unhook([obj], ticket, func)
    *   returns: true of success, false otherwise */
   int idx = 1;
   struct listener *l;
+  lua_handler lh;
 
   /* If the param is not as expected */
   if (!lua_islightuserdata(L, idx))
@@ -1163,9 +1170,12 @@ LuaUnRegEvent(lua_State *L)
     }
 
   l = (struct listener*)lua_touserdata(L, idx++);
+  /* the handler is used to validate the ticket.
+   * This is important, otherwise double unhook can crash screen. */
+  lh = LuaCheckHandler(L, idx, 0);
 
   /* Validate the listener structure */
-  if (!l || !l->handler)
+  if (lh == LUA_NOREF || !l || !l->handler)
     {
       /* invalid */
       lua_pushboolean(L,0);
index 54fa4eb..9d92d62 100644 (file)
@@ -1,20 +1,20 @@
 --[[ For now, this sample function will simply record all the commands executed ]]--
 
-ticket1 = screen.hook("cmdexecuted", function(name, args)
-      os.execute('mkdir -p /tmp/debug')
-      local f = io.open('/tmp/debug/22', 'a')
-      f:write("Command executed: " .. name)
+function cmd1(name, args)
+  os.execute('mkdir -p /tmp/debug')
+  local f = io.open('/tmp/debug/22', 'a')
+  f:write("Command executed: " .. name)
 
-      for i, c in pairs(args) do
-       f:write(" " .. c)
-      end
+  for i, c in pairs(args) do
+    f:write(" " .. c)
+  end
 
-      f:write("\n")
-      f:close()
-      return 0
-    end)
+  f:write("\n")
+  f:close()
+  return 0
+end
 
-function cmd(name, args)
+function cmd2(name, args)
     os.execute('mkdir -p /tmp/debug')
     local f = io.open('/tmp/debug/11', 'a')
     f:write("Command executed: " .. name)
@@ -28,17 +28,30 @@ function cmd(name, args)
     return 0
 end
 
-ticket2 = screen.hook("cmdexecuted", "cmd")
+ticket1 = screen.hook("cmdexecuted", cmd1)
+ticket2 = screen.hook("cmdexecuted", "cmd2")
 
 function unhook()
   if ticket1 ~= nil then
-    screen.unhook(ticket1)
+    screen.unhook(ticket1, cmd1)
     ticket1 = nil
   end
 
   if ticket2 ~= nil then
-    screen.unhook(ticket2)
+    screen.unhook(ticket2, "cmd2")
     ticket2 = nil
   end
 end
 
+--A second unhook should faild
+function debug_unhook()
+  if screen.unhook(ticket1, "cmd1") == false then
+    error("unhook failed")
+  end
+end
+    
+--A second hook should faild
+function debug_hook()
+  ticket1=screen.hook("cmdexecuted", "cmd1")
+end
+