From 08ed96dd7608a876dd4cdd939988b39892e73dbd Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 27 Dec 2020 21:40:12 +0200 Subject: [PATCH] Fix memory leaks in error replies due to recent change (#8249) Recently 4ef25c45b started using addReplyErrorSds in place of addReplySds the later takes ownership of the string but the former did not. This introduced memory leaks when a script returns an error to redis, and also in clusterRedirectClient (two new usages of addReplyErrorSds which was mostly unused till now. This commit chagnes two thanks. 1. change addReplyErrorSds to take ownership of the error string. 2. scripting.c doesn't actually need to use addReplyErrorSds, it's a perfect match for addReplyErrorFormat (replaces newlines with spaces) --- src/networking.c | 2 ++ src/scripting.c | 5 +---- src/server.c | 3 ++- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/networking.c b/src/networking.c index 24a601630..34e8b8481 100644 --- a/src/networking.c +++ b/src/networking.c @@ -458,9 +458,11 @@ void addReplyError(client *c, const char *err) { } /* See addReplyErrorLength for expectations from the input string. */ +/* As a side effect the SDS string is freed. */ void addReplyErrorSds(client *c, sds err) { addReplyErrorLength(c,err,sdslen(err)); afterErrorReply(c,err,sdslen(err)); + sdsfree(err); } /* See addReplyErrorLength for expectations from the formatted string. diff --git a/src/scripting.c b/src/scripting.c index 2ac8268ed..e75892046 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -366,10 +366,7 @@ void luaReplyToRedisReply(client *c, lua_State *lua) { lua_gettable(lua,-2); t = lua_type(lua,-1); if (t == LUA_TSTRING) { - sds err = sdsnew(lua_tostring(lua,-1)); - sdsmapchars(err,"\r\n"," ",2); - addReplyErrorSds(c,sdscatprintf(sdsempty(),"-%s",err)); - sdsfree(err); + addReplyErrorFormat(c,"-%s",lua_tostring(lua,-1)); lua_pop(lua,2); return; } diff --git a/src/server.c b/src/server.c index 90f669bdb..257a39f71 100644 --- a/src/server.c +++ b/src/server.c @@ -3689,10 +3689,11 @@ void rejectCommandFormat(client *c, const char *fmt, ...) { sdsmapchars(s, "\r\n", " ", 2); if (c->cmd && c->cmd->proc == execCommand) { execCommandAbort(c, s); + sdsfree(s); } else { + /* The following frees 's'. */ addReplyErrorSds(c, s); } - sdsfree(s); } /* Returns 1 for commands that may have key names in their arguments, but have