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

[Tor Lillqvist]

 loolwsd/LOOLWSD.cpp |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

New commits:
commit 32b33ee68503b1ff649cced8ffa1ef25fe988e08
Author: Tor Lillqvist <tml at collabora.com>
Date:   Tue Oct 18 08:06:16 2016 +0300

    Add logging of NumDocBrokers and the size of the DocBrokers array
    
    I fear they get can out of sync when HTTPCrashTest::killLoKitProcesses()
    kills a loolkit process, and this then causes HTTPWSError::testMaxDocuments()
    to fail. This is to help debugging that.
    
    Haven't fully understood what is going on yet. But one thing is sure:
    It is a bad idea to duplicate the same state information in two
    places, we shouldn't really use that separate NumDocBrokers
    variable.
    
    Probably also NumConnections tracks state that can easily be
    calculated from the data structures, but maybe NumConnections does not
    that easily get out of sync.

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 468135c..42c605f 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -176,6 +176,19 @@ static int careerSpanSeconds = 0;
 
 namespace {
 
+static void logNumDocBrokers(int lineNo)
+{
+    int size = 0;
+    int nonEmpty = 0;
+    for (auto& i : DocBrokers)
+    {
+        size++;
+        if (i.second->getPublicUri().toString() != "")
+            nonEmpty++;
+    }
+    Log::debug() << "line " << lineNo << ": NumDocBrokers=" << LOOLWSD::NumDocBrokers << " size: " << size << " of which non-empty: " << nonEmpty << Log::end;
+}
+
 static inline
 void shutdownLimitReached(WebSocket& ws)
 {
@@ -731,13 +744,17 @@ private:
             }
 
 #if MAX_DOCUMENTS > 0
+            std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
+            logNumDocBrokers(__LINE__);
             if (++LOOLWSD::NumDocBrokers > MAX_DOCUMENTS)
             {
                 --LOOLWSD::NumDocBrokers;
                 Log::error("Maximum number of open documents reached.");
+                logNumDocBrokers(__LINE__);
                 shutdownLimitReached(*ws);
                 return;
             }
+            DocBrokersLock.unlock();
 #endif
 
             // Set one we just created.
@@ -757,6 +774,7 @@ private:
                 DocBrokers.erase(docKey);
 #if MAX_DOCUMENTS > 0
                 --LOOLWSD::NumDocBrokers;
+                logNumDocBrokers(__LINE__);
 #endif
             }
 
@@ -873,6 +891,7 @@ private:
                 DocBrokers.erase(docKey);
 #if MAX_DOCUMENTS > 0
                 --LOOLWSD::NumDocBrokers;
+                logNumDocBrokers(__LINE__);
 #endif
                 Log::info("Removing complete doc [" + docKey + "] from Admin.");
                 Admin::instance().rmDoc(docKey);