From 4bf9beb484ecc83330a99c76509dba47733ebcbf Mon Sep 17 00:00:00 2001 From: John Sully Date: Sun, 16 Feb 2020 03:33:45 -0500 Subject: [PATCH 1/3] aeDeleteEventLoop use after free and leak fixes Former-commit-id: 2fd93c5789a4e81455d51b2a4786f708e8d6a2d7 --- src/ae.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ae.cpp b/src/ae.cpp index b92cd4a67..16ac3ebba 100644 --- a/src/ae.cpp +++ b/src/ae.cpp @@ -390,10 +390,18 @@ extern "C" void aeDeleteEventLoop(aeEventLoop *eventLoop) { aeApiFree(eventLoop); zfree(eventLoop->events); zfree(eventLoop->fired); - zfree(eventLoop); fastlock_free(&eventLoop->flock); close(eventLoop->fdCmdRead); close(eventLoop->fdCmdWrite); + + auto *te = eventLoop->timeEventHead; + while (te) + { + auto *teNext = te->next; + zfree(te); + te = teNext; + } + zfree(eventLoop); } extern "C" void aeStop(aeEventLoop *eventLoop) { From 41c75234bd592de8cb531188767a2964596a829c Mon Sep 17 00:00:00 2001 From: John Sully Date: Sun, 16 Feb 2020 03:43:29 -0500 Subject: [PATCH 2/3] Fix memory leak of ReplicaNestState on shutdown Former-commit-id: 4781eda7225c2640e25387663c33ef74cd98b0c4 --- src/replication.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/replication.cpp b/src/replication.cpp index 9b8570c49..ba2008c0a 100644 --- a/src/replication.cpp +++ b/src/replication.cpp @@ -3371,7 +3371,7 @@ private: redisMaster *m_mi = nullptr; }; -static thread_local ReplicaNestState *s_pstate = nullptr; +static thread_local std::unique_ptr s_pstate; bool FInReplicaReplay() { @@ -3384,7 +3384,7 @@ static std::unordered_map g_mapmvcc; void replicaReplayCommand(client *c) { if (s_pstate == nullptr) - s_pstate = new (MALLOC_LOCAL) ReplicaNestState; + s_pstate = std::make_unique(); // the replay command contains two arguments: // 1: The UUID of the source From 8f6f496c7e64fa50f72777d9ec290da85e77e722 Mon Sep 17 00:00:00 2001 From: John Sully Date: Sun, 16 Feb 2020 17:08:00 -0500 Subject: [PATCH 3/3] Memory leak fix on config, and redisDb dtor Former-commit-id: b92bbf4de8ffc3edc965e2f9da4dd82ed7071559 --- src/config.cpp | 2 +- src/db.cpp | 8 ++++++++ src/server.h | 7 +++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 2aaad825e..2c9a9d518 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -405,7 +405,7 @@ void loadServerConfigFromString(char *config) { } else if ((!strcasecmp(argv[0],"slaveof") || !strcasecmp(argv[0],"replicaof")) && argc == 3) { slaveof_linenum = linenum; - replicationAddMaster(sdsnew(argv[1]), atoi(argv[2])); + replicationAddMaster(argv[1], atoi(argv[2])); } else if ((!strcasecmp(argv[0],"repl-ping-slave-period") || !strcasecmp(argv[0],"repl-ping-replica-period")) && argc == 2) diff --git a/src/db.cpp b/src/db.cpp index 9190d22f1..e94e0cdb1 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1305,6 +1305,14 @@ void setExpire(client *c, redisDb *db, robj *key, robj *subkey, long long when) rememberSlaveKeyWithExpire(db,key); } +redisDb::~redisDb() +{ + dictRelease(watched_keys); + dictRelease(ready_keys); + dictRelease(blocking_keys); + listRelease(defrag_later); +} + void setExpire(client *c, redisDb *db, robj *key, expireEntry &&e) { dictEntry *kde; diff --git a/src/server.h b/src/server.h index 3ec2e8948..135a216ae 100644 --- a/src/server.h +++ b/src/server.h @@ -1127,10 +1127,13 @@ typedef struct clientReplyBlock { /* Redis database representation. There are multiple databases identified * by integers from 0 (the default database) up to the max configured * database. The database number is the 'id' field in the structure. */ -typedef struct redisDb { +struct redisDb { redisDb() : expireitr(nullptr) {}; + + ~redisDb(); + dict *pdict; /* The keyspace for this DB */ expireset *setexpire; expireset::setiter expireitr; @@ -1142,7 +1145,7 @@ typedef struct redisDb { long long last_expire_set; /* when the last expire was set */ double avg_ttl; /* Average TTL, just for stats */ list *defrag_later; /* List of key names to attempt to defrag one by one, gradually. */ -} redisDb; +}; /* Client MULTI/EXEC state */ typedef struct multiCmd {