diff options
-rw-r--r-- | src/script.c | 10 | ||||
-rw-r--r-- | src/script.h | 2 | ||||
-rw-r--r-- | src/script_lua.c | 87 | ||||
-rw-r--r-- | tests/unit/scripting.tcl | 15 |
4 files changed, 88 insertions, 26 deletions
diff --git a/src/script.c b/src/script.c index 57fa7eeba..2a770fa61 100644 --- a/src/script.c +++ b/src/script.c @@ -513,22 +513,18 @@ static int scriptVerifyAllowStale(client *c, sds *err) { * up to the engine to take and parse. * The err out variable is set only if error occurs and describe the error. * If err is set on reply is written to the run_ctx client. */ -void scriptCall(scriptRunCtx *run_ctx, robj* *argv, int argc, sds *err) { +void scriptCall(scriptRunCtx *run_ctx, sds *err) { client *c = run_ctx->c; /* Setup our fake client for command execution */ - c->argv = argv; - c->argc = argc; c->user = run_ctx->original_client->user; /* Process module hooks */ moduleCallCommandFilters(c); - argv = c->argv; - argc = c->argc; - struct redisCommand *cmd = lookupCommand(argv, argc); + struct redisCommand *cmd = lookupCommand(c->argv, c->argc); c->cmd = c->lastcmd = c->realcmd = cmd; - if (scriptVerifyCommandArity(cmd, argc, err) != C_OK) { + if (scriptVerifyCommandArity(cmd, c->argc, err) != C_OK) { goto error; } diff --git a/src/script.h b/src/script.h index b601342c5..b892637ff 100644 --- a/src/script.h +++ b/src/script.h @@ -98,7 +98,7 @@ int scriptPrepareForRun(scriptRunCtx *r_ctx, client *engine_client, client *call void scriptResetRun(scriptRunCtx *r_ctx); int scriptSetResp(scriptRunCtx *r_ctx, int resp); int scriptSetRepl(scriptRunCtx *r_ctx, int repl); -void scriptCall(scriptRunCtx *r_ctx, robj **argv, int argc, sds *err); +void scriptCall(scriptRunCtx *r_ctx, sds *err); int scriptInterrupt(scriptRunCtx *r_ctx); void scriptKill(client *c, int is_eval); int scriptIsRunning(); diff --git a/src/script_lua.c b/src/script_lua.c index 578a22067..b22f9e5b9 100644 --- a/src/script_lua.c +++ b/src/script_lua.c @@ -783,6 +783,17 @@ static void luaReplyToRedisReply(client *c, client* script_client, lua_State *lu /* --------------------------------------------------------------------------- * Lua redis.* functions implementations. * ------------------------------------------------------------------------- */ +void freeLuaRedisArgv(robj **argv, int argc); + +/* Cached argv array across calls. */ +static robj **lua_argv = NULL; +static int lua_argv_size = 0; + +/* Cache of recently used small arguments to avoid malloc calls. */ +#define LUA_CMD_OBJCACHE_SIZE 32 +#define LUA_CMD_OBJCACHE_MAX_LEN 64 +static robj *lua_args_cached_objects[LUA_CMD_OBJCACHE_SIZE]; +static size_t lua_args_cached_objects_len[LUA_CMD_OBJCACHE_SIZE]; static robj **luaArgsToRedisArgv(lua_State *lua, int *argc) { int j; @@ -793,8 +804,11 @@ static robj **luaArgsToRedisArgv(lua_State *lua, int *argc) { return NULL; } - /* Build the arguments vector */ - robj **argv = zcalloc(sizeof(robj*) * *argc); + /* Build the arguments vector (reuse a cached argv from last call) */ + if (lua_argv_size < *argc) { + lua_argv = zrealloc(lua_argv,sizeof(robj*)* *argc); + lua_argv_size = *argc; + } for (j = 0; j < *argc; j++) { char *obj_s; @@ -812,8 +826,18 @@ static robj **luaArgsToRedisArgv(lua_State *lua, int *argc) { obj_s = (char*)lua_tolstring(lua,j+1,&obj_len); if (obj_s == NULL) break; /* Not a string. */ } - - argv[j] = createStringObject(obj_s, obj_len); + /* Try to use a cached object. */ + if (j < LUA_CMD_OBJCACHE_SIZE && lua_args_cached_objects[j] && + lua_args_cached_objects_len[j] >= obj_len) + { + sds s = lua_args_cached_objects[j]->ptr; + lua_argv[j] = lua_args_cached_objects[j]; + lua_args_cached_objects[j] = NULL; + memcpy(s,obj_s,obj_len+1); + sdssetlen(s, obj_len); + } else { + lua_argv[j] = createStringObject(obj_s, obj_len); + } } /* Pop all arguments from the stack, we do not need them anymore @@ -824,17 +848,42 @@ static robj **luaArgsToRedisArgv(lua_State *lua, int *argc) { * is not a string or an integer (lua_isstring() return true for * integers as well). */ if (j != *argc) { - j--; - while (j >= 0) { - decrRefCount(argv[j]); - j--; - } - zfree(argv); + freeLuaRedisArgv(lua_argv, j); luaPushError(lua, "Lua redis lib command arguments must be strings or integers"); return NULL; } - return argv; + return lua_argv; +} + +void freeLuaRedisArgv(robj **argv, int argc) { + int j; + for (j = 0; j < argc; j++) { + robj *o = argv[j]; + + /* Try to cache the object in the lua_args_cached_objects array. + * The object must be small, SDS-encoded, and with refcount = 1 + * (we must be the only owner) for us to cache it. */ + if (j < LUA_CMD_OBJCACHE_SIZE && + o->refcount == 1 && + (o->encoding == OBJ_ENCODING_RAW || + o->encoding == OBJ_ENCODING_EMBSTR) && + sdslen(o->ptr) <= LUA_CMD_OBJCACHE_MAX_LEN) + { + sds s = o->ptr; + if (lua_args_cached_objects[j]) decrRefCount(lua_args_cached_objects[j]); + lua_args_cached_objects[j] = o; + lua_args_cached_objects_len[j] = sdsalloc(s); + } else { + decrRefCount(o); + } + } + if (argv != lua_argv) { + /* The command changed argv, scrap the cache and start over. */ + zfree(argv); + lua_argv = NULL; + lua_argv_size = 0; + } } static int luaRedisGenericCommand(lua_State *lua, int raise_error) { @@ -845,9 +894,9 @@ static int luaRedisGenericCommand(lua_State *lua, int raise_error) { client* c = rctx->c; sds reply; - int argc; - robj **argv = luaArgsToRedisArgv(lua, &argc); - if (argv == NULL) { + c->argv = luaArgsToRedisArgv(lua, &c->argc); + c->argv_len = lua_argv_size; + if (c->argv == NULL) { return raise_error ? luaError(lua) : 1; } @@ -883,7 +932,7 @@ static int luaRedisGenericCommand(lua_State *lua, int raise_error) { ldbLog(cmdlog); } - scriptCall(rctx, argv, argc, &err); + scriptCall(rctx, &err); if (err) { luaPushError(lua, err); sdsfree(err); @@ -928,8 +977,11 @@ static int luaRedisGenericCommand(lua_State *lua, int raise_error) { cleanup: /* Clean up. Command code may have changed argv/argc so we use the * argv/argc of the client instead of the local variables. */ - freeClientArgv(c); + freeLuaRedisArgv(c->argv, c->argc); + c->argc = c->argv_len = 0; c->user = NULL; + c->argv = NULL; + freeClientArgv(c); inuse--; if (raise_error) { @@ -1096,8 +1148,7 @@ static int luaRedisAclCheckCmdPermissionsCommand(lua_State *lua) { } } - while (argc--) decrRefCount(argv[argc]); - zfree(argv); + freeLuaRedisArgv(argv, argc); if (raise_error) return luaError(lua); else diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl index ca9422787..fa617d7fe 100644 --- a/tests/unit/scripting.tcl +++ b/tests/unit/scripting.tcl @@ -1956,5 +1956,20 @@ start_server {tags {"scripting"}} { r eval {local status, res = pcall(function() return foo end); return 'status: ' .. tostring(status) .. ' result: ' .. res} 0 ] } + + test "LUA test pcall with non string/integer arg" { + assert_error "ERR Lua redis lib command arguments must be strings or integers*" { + r eval { + local x={} + return redis.call("ping", x) + } 0 + } + # run another command, to make sure the cached argv array survived + assert_equal [ + r eval { + return redis.call("ping", "asdf") + } 0 + ] {asdf} + } } |