From 71b4a7b6d31db16a2cfe988099d9ab49fbde6368 Mon Sep 17 00:00:00 2001 From: christianEQ Date: Mon, 30 Nov 2020 21:56:20 +0000 Subject: [PATCH 01/10] Issue: #204 Allocate 8 MB to thread stack Former-commit-id: 66a41dafc47a20251f5f6776625780dfa26ee505 --- src/server.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/server.cpp b/src/server.cpp index 92241f02b..3779a1ff0 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -5788,9 +5788,13 @@ int main(int argc, char **argv) { setOOMScoreAdj(-1); serverAssert(cserver.cthreads > 0 && cserver.cthreads <= MAX_EVENT_LOOPS); pthread_t rgthread[MAX_EVENT_LOOPS]; + + pthread_attr_t tattr; + pthread_attr_init(&tattr); + pthread_attr_setstacksize(&tattr, 1 << 23); // 8 MB for (int iel = 0; iel < cserver.cthreads; ++iel) { - pthread_create(rgthread + iel, NULL, workerThreadMain, (void*)((int64_t)iel)); + pthread_create(rgthread + iel, &tattr, workerThreadMain, (void*)((int64_t)iel)); if (cserver.fThreadAffinity) { #ifdef __linux__ From 63d79cc2979bc36ee5064190a3a13db74012cd4b Mon Sep 17 00:00:00 2001 From: Kajaruban Surendran Date: Fri, 4 Dec 2020 16:27:01 +0000 Subject: [PATCH 02/10] Calling initServerThread function cserver.cthreads(server-threads) times than MAX_EVENT_LOOPS(constant) Former-commit-id: d6b4f994ad776efab6d647947fe1543359ec9401 --- src/server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.cpp b/src/server.cpp index 3779a1ff0..f0ef6b016 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -5695,7 +5695,7 @@ int main(int argc, char **argv) { validateConfiguration(); - for (int iel = 0; iel < MAX_EVENT_LOOPS; ++iel) + for (int iel = 0; iel < cserver.cthreads; ++iel) { initServerThread(g_pserver->rgthreadvar+iel, iel == IDX_EVENT_LOOP_MAIN); } From 98f0c6b599526e724c6485dfb86374ae1622d1c9 Mon Sep 17 00:00:00 2001 From: Kajaruban Surendran Date: Mon, 23 Nov 2020 21:58:54 +0000 Subject: [PATCH 03/10] Call aePostFunction to change the setsize of each threads by that thread Former-commit-id: 7b1221a74d06616149436fd44d67a2ad83048e44 --- src/config.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 9aa1e0990..fbd440dfb 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -2278,10 +2278,12 @@ static int updateMaxclients(long long val, long long prev, const char **err) { if ((unsigned int) aeGetSetSize(g_pserver->rgthreadvar[iel].el) < g_pserver->maxclients + CONFIG_FDSET_INCR) { - if (aeResizeSetSize(g_pserver->rgthreadvar[iel].el, - g_pserver->maxclients + CONFIG_FDSET_INCR) == AE_ERR) - { - *err = "The event loop API used by Redis is not able to handle the specified number of clients"; + int res = aePostFunction(g_pserver->rgthreadvar[iel].el, [iel] { + aeResizeSetSize(g_pserver->rgthreadvar[iel].el, g_pserver->maxclients + CONFIG_FDSET_INCR); + }); + + if (res != AE_OK){ + *err = "Failed to set the setsize for this thread."; return 0; } } From 024739dee7775e7155e424950a61f6b08df05d40 Mon Sep 17 00:00:00 2001 From: Kajaruban Surendran Date: Wed, 25 Nov 2020 19:29:10 +0000 Subject: [PATCH 04/10] Adding thread index to the error message Former-commit-id: e02120cdf7ca5adec58aa8e84aece89b01d6b751 --- src/config.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index fbd440dfb..6c49d628b 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -2278,12 +2278,14 @@ static int updateMaxclients(long long val, long long prev, const char **err) { if ((unsigned int) aeGetSetSize(g_pserver->rgthreadvar[iel].el) < g_pserver->maxclients + CONFIG_FDSET_INCR) { - int res = aePostFunction(g_pserver->rgthreadvar[iel].el, [iel] { + int res = aePostFunction(g_pserver->rgthreadvar[iel].el, [iel] { aeResizeSetSize(g_pserver->rgthreadvar[iel].el, g_pserver->maxclients + CONFIG_FDSET_INCR); }); if (res != AE_OK){ - *err = "Failed to set the setsize for this thread."; + static char msg[128]; + sprintf(msg, "Failed to post the request to change setsize for Thread %d", iel); + *err = msg; return 0; } } From 123aec9beda1116ec836851e2cf1052edbe09d5f Mon Sep 17 00:00:00 2001 From: Kajaruban Surendran Date: Thu, 26 Nov 2020 18:39:05 +0000 Subject: [PATCH 05/10] check the aeResizeSetSize return code Former-commit-id: 572d41325b9fd5322ab0e41f88e384d69f52f41d --- src/config.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/config.cpp b/src/config.cpp index 6c49d628b..88939b001 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -2279,7 +2279,9 @@ static int updateMaxclients(long long val, long long prev, const char **err) { g_pserver->maxclients + CONFIG_FDSET_INCR) { int res = aePostFunction(g_pserver->rgthreadvar[iel].el, [iel] { - aeResizeSetSize(g_pserver->rgthreadvar[iel].el, g_pserver->maxclients + CONFIG_FDSET_INCR); + if (aeResizeSetSize(g_pserver->rgthreadvar[iel].el, g_pserver->maxclients + CONFIG_FDSET_INCR) == AE_ERR) { + serverPanic("Failed to change the setsize for Thread %d", iel); + } }); if (res != AE_OK){ From 5886c1b985c04dc74a39e1cb207bea28705b3440 Mon Sep 17 00:00:00 2001 From: Kajaruban Surendran Date: Mon, 30 Nov 2020 20:20:37 +0000 Subject: [PATCH 06/10] Handling of aeResizeSetSize's return code for current thread Former-commit-id: c25fb3d74fb9e7adb6ad7ef730355e325e982cd2 --- src/ae.cpp | 5 +++++ src/ae.h | 1 + src/config.cpp | 22 +++++++++++++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/ae.cpp b/src/ae.cpp index cb500d7b6..0d9bf21e7 100644 --- a/src/ae.cpp +++ b/src/ae.cpp @@ -373,6 +373,11 @@ int aeGetSetSize(aeEventLoop *eventLoop) { return eventLoop->setsize; } +/* Return the current EventLoop. */ +aeEventLoop *aeGetCurrentEventLoop(){ + return g_eventLoopThisThread; +} + /* Tells the next iteration/s of the event processing to set timeout of 0. */ void aeSetDontWait(aeEventLoop *eventLoop, int noWait) { if (noWait) diff --git a/src/ae.h b/src/ae.h index fdd444d3a..e77abb01f 100644 --- a/src/ae.h +++ b/src/ae.h @@ -160,6 +160,7 @@ const char *aeGetApiName(void); void aeSetBeforeSleepProc(aeEventLoop *eventLoop, aeBeforeSleepProc *beforesleep, int flags); void aeSetAfterSleepProc(aeEventLoop *eventLoop, aeBeforeSleepProc *aftersleep, int flags); int aeGetSetSize(aeEventLoop *eventLoop); +aeEventLoop *aeGetCurrentEventLoop(); int aeResizeSetSize(aeEventLoop *eventLoop, int setsize); void aeSetDontWait(aeEventLoop *eventLoop, int noWait); diff --git a/src/config.cpp b/src/config.cpp index 88939b001..e00fdf641 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -2273,14 +2273,33 @@ static int updateMaxclients(long long val, long long prev, const char **err) { } return 0; } + /* Change the SetSize for the current thread first. + * If any error, return the error message to the client, otherwise, continue to do the same for other threads */ + if ((unsigned int) aeGetSetSize(aeGetCurrentEventLoop()) < + g_pserver->maxclients + CONFIG_FDSET_INCR) + { + if (aeResizeSetSize(aeGetCurrentEventLoop(), + g_pserver->maxclients + CONFIG_FDSET_INCR) == AE_ERR) + { + *err = "The event loop API used by Redis is not able to handle the specified number of clients"; + return 0; + } + serverLog(LL_DEBUG,"Successfully changed the setsize for current thread %d", ielFromEventLoop(aeGetCurrentEventLoop())); + } + for (int iel = 0; iel < cserver.cthreads; ++iel) { + if (g_pserver->rgthreadvar[iel].el == aeGetCurrentEventLoop()) + { + continue; + } + if ((unsigned int) aeGetSetSize(g_pserver->rgthreadvar[iel].el) < g_pserver->maxclients + CONFIG_FDSET_INCR) { int res = aePostFunction(g_pserver->rgthreadvar[iel].el, [iel] { if (aeResizeSetSize(g_pserver->rgthreadvar[iel].el, g_pserver->maxclients + CONFIG_FDSET_INCR) == AE_ERR) { - serverPanic("Failed to change the setsize for Thread %d", iel); + serverLog(LL_WARNING,"Failed to change the setsize for Thread %d", iel); } }); @@ -2290,6 +2309,7 @@ static int updateMaxclients(long long val, long long prev, const char **err) { *err = msg; return 0; } + serverLog(LL_DEBUG,"Successfully post the request to change the setsize for thread %d", iel); } } } From d46ed5d7706661af5d1a78349f2b123a990dd404 Mon Sep 17 00:00:00 2001 From: Kajaruban Surendran Date: Tue, 1 Dec 2020 21:49:05 +0000 Subject: [PATCH 07/10] Addressing PR comments Former-commit-id: 63e5d00bdfdb054353c7f3fb7a5890d98e7a7f92 --- src/config.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index e00fdf641..1e5774071 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -2273,8 +2273,8 @@ static int updateMaxclients(long long val, long long prev, const char **err) { } return 0; } - /* Change the SetSize for the current thread first. - * If any error, return the error message to the client, otherwise, continue to do the same for other threads */ + /* Change the SetSize for the current thread first. If any error, return the error message to the client, + * otherwise, continue to do the same for other threads */ if ((unsigned int) aeGetSetSize(aeGetCurrentEventLoop()) < g_pserver->maxclients + CONFIG_FDSET_INCR) { From 8586dfb15c39934596e043d37f102cb0671fa864 Mon Sep 17 00:00:00 2001 From: Kajaruban Surendran Date: Wed, 2 Dec 2020 00:27:39 +0000 Subject: [PATCH 08/10] Addressing PR comments Former-commit-id: bdd14ac517fb1921dd6c398ad10346c74324740d --- src/config.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 1e5774071..cf8a31bcb 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -2276,7 +2276,7 @@ static int updateMaxclients(long long val, long long prev, const char **err) { /* Change the SetSize for the current thread first. If any error, return the error message to the client, * otherwise, continue to do the same for other threads */ if ((unsigned int) aeGetSetSize(aeGetCurrentEventLoop()) < - g_pserver->maxclients + CONFIG_FDSET_INCR) + g_pserver->maxclients + CONFIG_FDSET_INCR) { if (aeResizeSetSize(aeGetCurrentEventLoop(), g_pserver->maxclients + CONFIG_FDSET_INCR) == AE_ERR) @@ -2284,13 +2284,12 @@ static int updateMaxclients(long long val, long long prev, const char **err) { *err = "The event loop API used by Redis is not able to handle the specified number of clients"; return 0; } - serverLog(LL_DEBUG,"Successfully changed the setsize for current thread %d", ielFromEventLoop(aeGetCurrentEventLoop())); + serverLog(LL_DEBUG,"Successfully changed the setsize for current thread %d", ielFromEventLoop(aeGetCurrentEventLoop())); } for (int iel = 0; iel < cserver.cthreads; ++iel) { - if (g_pserver->rgthreadvar[iel].el == aeGetCurrentEventLoop()) - { + if (g_pserver->rgthreadvar[iel].el == aeGetCurrentEventLoop()){ continue; } @@ -2309,7 +2308,7 @@ static int updateMaxclients(long long val, long long prev, const char **err) { *err = msg; return 0; } - serverLog(LL_DEBUG,"Successfully post the request to change the setsize for thread %d", iel); + serverLog(LL_DEBUG,"Successfully post the request to change the setsize for thread %d", iel); } } } From 51ef4343c40676f10a65cd9ec6533b725f90f3cb Mon Sep 17 00:00:00 2001 From: John Sully Date: Fri, 11 Dec 2020 03:59:26 +0000 Subject: [PATCH 09/10] Prevent use after free Former-commit-id: 81573de393eef083d2209117b270d088a7b3f819 --- src/server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.cpp b/src/server.cpp index 92241f02b..562a65ccc 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -4025,8 +4025,8 @@ bool client::postFunction(std::function fn, bool fLock) { this->casyncOpsPending++; return aePostFunction(g_pserver->rgthreadvar[this->iel].el, [this, fn]{ std::lock_guardlock)> lock(this->lock); - --casyncOpsPending; fn(this); + --casyncOpsPending; }, false, fLock) == AE_OK; } From 59a1d5085fb958cc1cca8ba5890f75146781bc8c Mon Sep 17 00:00:00 2001 From: John Sully Date: Fri, 11 Dec 2020 03:59:39 +0000 Subject: [PATCH 10/10] don't try and free a client that can't be freed Former-commit-id: 7d3c5f1e64a79f47a103ce97c6991aa473fb697a --- src/networking.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/networking.cpp b/src/networking.cpp index 465376eeb..5241a3f70 100644 --- a/src/networking.cpp +++ b/src/networking.cpp @@ -1542,7 +1542,7 @@ int freeClientsInAsyncFreeQueue(int iel) { while((ln = listNext(&li))) { client *c = (client*)listNodeValue(ln); - if (c->iel == iel && !(c->flags & CLIENT_PROTECTED)) + if (c->iel == iel && !(c->flags & CLIENT_PROTECTED) && !c->casyncOpsPending) { vecclientsFree.push_back(c); listDelNode(g_pserver->clients_to_close, ln);