From 41d3147344da4656b6a21c73373c34d25fe8f24d Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 29 Aug 2013 11:49:23 +0200 Subject: [PATCH] Fixed critical memory leak from EVAL. Multiple missing calls to lua_pop prevented the error handler function pushed on the stack for lua_pcall() to be popped before returning, causing a memory leak in almost all the code paths of EVAL (both successful calls and calls returning errors). This caused two issues: Lua leaking memory (and this was very visible from INFO memory output, as the 'used_memory_lua' field reported an always increasing amount of memory used), and as a result slower and slower GC cycles resulting in all the CPU being used. Thanks to Tanguy Le Barzic for noticing something was wrong with his 2.8 slave, and for creating a testing EC2 environment where I was able to investigate the issue. --- src/scripting.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/scripting.c b/src/scripting.c index baf58527..ac1a913f 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -876,7 +876,12 @@ void evalGenericCommand(redisClient *c, int evalsha) { addReply(c, shared.noscripterr); return; } - if (luaCreateFunction(c,lua,funcname,c->argv[1]) == REDIS_ERR) return; + if (luaCreateFunction(c,lua,funcname,c->argv[1]) == REDIS_ERR) { + lua_pop(lua,1); /* remove the error handler from the stack. */ + /* The error is sent to the client by luaCreateFunction() + * itself when it returns REDIS_ERR. */ + return; + } /* Now the following is guaranteed to return non nil */ lua_getglobal(lua, funcname); redisAssert(!lua_isnil(lua,-1)); @@ -905,7 +910,6 @@ void evalGenericCommand(redisClient *c, int evalsha) { /* At this point whether this script was never seen before or if it was * already defined, we can call it. We have zero arguments and expect * a single return value. */ - err = lua_pcall(lua,0,1,-2); /* Perform some cleanup that we need to do both on error and success. */ @@ -924,11 +928,12 @@ void evalGenericCommand(redisClient *c, int evalsha) { if (err) { addReplyErrorFormat(c,"Error running script (call to %s): %s\n", funcname, lua_tostring(lua,-1)); - lua_pop(lua,1); /* Consume the Lua reply. */ + lua_pop(lua,2); /* Consume the Lua reply and remove error handler. */ } else { /* On success convert the Lua return value into Redis protocol, and * send it to * the client. */ - luaReplyToRedisReply(c,lua); + luaReplyToRedisReply(c,lua); /* Convert and consume the reply. */ + lua_pop(lua,1); /* Remove the error handler. */ } /* EVALSHA should be propagated to Slave and AOF file as full EVAL, unless