From 8f95b3c7d3ed9fc736fd2cfdfd9cfb3d075e3438 Mon Sep 17 00:00:00 2001 From: Rui Guo Date: Fri, 12 Jun 2009 22:22:58 +0800 Subject: [PATCH] Fix refcount logic & duplicate unhook checking. 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 | 88 +++++++++++++++++++++++++-------------------- src/scripts/cmdcallback.lua | 43 ++++++++++++++-------- 2 files changed, 77 insertions(+), 54 deletions(-) diff --git a/src/lua.c b/src/lua.c index 9357a9e..a5a53a0 100644 --- 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); diff --git a/src/scripts/cmdcallback.lua b/src/scripts/cmdcallback.lua index 54fa4eb..9d92d62 100644 --- a/src/scripts/cmdcallback.lua +++ b/src/scripts/cmdcallback.lua @@ -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 + -- 2.11.4.GIT