[Libreoffice-commits] online.git: loolwsd/LOOLKit.cpp

[Ashod Nakashian]

 loolwsd/LOOLKit.cpp |   30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

New commits:
commit 4dc9fa7eb7563ff1d2013ad6d3bd22d3512f0cf9
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Fri Jan 22 09:27:03 2016 -0500

    loolwsd: prevent deadlock when purging sessions
    
    Change-Id: I293e28674ed721741fa0afc57b37a636833d2e0e
    Reviewed-on: https://gerrit.libreoffice.org/21750
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index e474efa..5fac3eb 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -581,21 +581,31 @@ public:
     /// the remaining number of clients.
     size_t purgeSessions()
     {
-        std::unique_lock<std::recursive_mutex> lock(_mutex);
-
-        for (auto it =_connections.cbegin(); it != _connections.cend(); )
+        std::vector<std::shared_ptr<ChildProcessSession>> deadSessions;
         {
-            if (!it->second->isRunning())
-            {
-                onUnload(it->second->getSession()->getId());
-                it = _connections.erase(it);
-            }
-            else
+            std::unique_lock<std::recursive_mutex> lock(_mutex);
+
+            for (auto it =_connections.cbegin(); it != _connections.cend(); )
             {
-                ++it;
+                if (!it->second->isRunning())
+                {
+                    deadSessions.push_back(it->second->getSession());
+                    it = _connections.erase(it);
+                }
+                else
+                {
+                    ++it;
+                }
             }
         }
 
+        // Don't destroy sessions while holding our lock.
+        // We may deadlock if a session is waiting on us
+        // during callback initiated while handling a command
+        // and the dtor tries to take its lock (which is taken).
+        deadSessions.clear();
+
+        std::unique_lock<std::recursive_mutex> lock(_mutex);
         return _connections.size();
     }