From 4d5d7ed59f3da3292696991c96d6ec59b6db30e7 Mon Sep 17 00:00:00 2001
From: John Sully <john@csquare.ca>
Date: Thu, 19 Mar 2020 15:28:39 -0400
Subject: [PATCH] Fix lock inversion in processEventsWhileBlocked

Former-commit-id: a9249d4a82a0f0355ac8ffa40b34b9c14cabf66b
---
 src/networking.cpp | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/networking.cpp b/src/networking.cpp
index f6425af60..94b202c17 100644
--- a/src/networking.cpp
+++ b/src/networking.cpp
@@ -1630,12 +1630,12 @@ void sendReplyToClient(aeEventLoop *el, int fd, void *privdata, int mask) {
     if (writeToClient(fd,c,1) == C_ERR)
     {
         AeLocker ae;
-        c->lock();
+        c->lock.lock();
         ae.arm(c);
         if (c->flags & CLIENT_CLOSE_ASAP)
         {
             if (!freeClient(c))
-                c->unlock();
+                c->lock.unlock();
         }
     }
 }
@@ -3107,12 +3107,23 @@ int processEventsWhileBlocked(int iel) {
     int iterations = 4; /* See the function top-comment. */
     int count = 0;
 
-    client *c = serverTL->current_client;
-    if (c != nullptr)
+    std::vector<client*> vecclients;
+    listIter li;
+    listNode *ln;
+    listRewind(g_pserver->clients, &li);
+
+    // All client locks must be acquired *after* the global lock is reacquired to prevent deadlocks
+    //  so unlock here, and save them for reacquisition later
+    while ((ln = listNext(&li)) != nullptr)
     {
-        serverAssert(c->flags & CLIENT_PROTECTED);
-        c->lock.unlock();
+        client *c = (client*)listNodeValue(ln);
+        if (c->lock.fOwnLock()) {
+            serverAssert(c->flags & CLIENT_PROTECTED);  // If the client is not protected we have no gurantee they won't be free'd in the event loop
+            c->lock.unlock();
+            vecclients.push_back(c);
+        }
     }
+    
 
     aeReleaseLock();
     try
@@ -3129,18 +3140,18 @@ int processEventsWhileBlocked(int iel) {
     {
         // Caller expects us to be locked so fix and rethrow
         AeLocker locker;
-        if (c != nullptr)
-            c->lock.lock();
-        locker.arm(c);
+        locker.arm(nullptr);
         locker.release();
+        for (client *c : vecclients)
+            c->lock.lock();
         throw;
     }
     
     AeLocker locker;
-    if (c != nullptr)
-        c->lock.lock();
-    locker.arm(c);
+    locker.arm(nullptr);
     locker.release();
+    for (client *c : vecclients)
+        c->lock.lock();
     return count;
 }