[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4-0' - wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

[Michael Meeks (via logerrit)]

 wsd/DocumentBroker.cpp |   13 +++++++++++--
 wsd/DocumentBroker.hpp |    8 +++++++-
 wsd/LOOLWSD.cpp        |    7 ++++---
 3 files changed, 22 insertions(+), 6 deletions(-)

New commits:
commit c1f6b75776a450866cb2f426ea631abb0bb6582b
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Fri Oct 4 10:30:49 2019 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Fri Oct 4 17:23:58 2019 +0200

    belt & braces fix erroneous popup of limit dialog
    
    The shared_ptr allows the DocumentBroker's and ConvertToBroker to linger
    after they are removed from the list, making ConvertToBroker::getInstanceCount
    potentially larger than the number of documents transiently. Fix this
    with a dispose method called on list removal.
    Also make the arithmetic signed, to avoid unfortunate wrapping.
    Also when the limit is large, don't show a message whatever happens.
    
    Change-Id: Id2571429de48ae75e851c3fdc49e24a02aaaf6e9
    Reviewed-on: https://gerrit.libreoffice.org/80193
    Reviewed-by: Aron Budea <aron.budea at collabora.com>
    Tested-by: Aron Budea <aron.budea at collabora.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 253774249..dae6da707 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1938,10 +1938,19 @@ ConvertToBroker::ConvertToBroker(const std::string& uri,
     NumConverters++;
 }
 
+void ConvertToBroker::dispose()
+{
+    if (!_uriOrig.empty())
+    {
+        NumConverters--;
+        removeFile(_uriOrig);
+        _uriOrig.clear();
+    }
+}
+
 ConvertToBroker::~ConvertToBroker()
 {
-    NumConverters--;
-    removeFile(_uriOrig);
+    dispose();
 }
 
 void ConvertToBroker::removeFile(const std::string &uriOrig)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 55216a0a9..24a5e1ac9 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -225,6 +225,9 @@ public:
 
     virtual ~DocumentBroker();
 
+    /// Called when removed from the DocBrokers list
+    virtual void dispose() {}
+
     /// Start processing events
     void startThread();
 
@@ -403,7 +406,7 @@ private:
     void getIOStats(uint64_t &sent, uint64_t &recv);
 
 protected:
-    const std::string _uriOrig;
+    std::string _uriOrig;
 private:
     const Poco::URI _uriPublic;
     /// URL-based key. May be repeated during the lifetime of WSD.
@@ -482,6 +485,9 @@ public:
                     const std::string& docKey);
     virtual ~ConvertToBroker();
 
+    /// Called when removed from the DocBrokers list
+    void dispose() override;
+
     /// How many live conversions are running.
     static size_t getInstanceCount();
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 8eb9e31ca..9a1a46e92 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -255,8 +255,9 @@ inline void shutdownLimitReached(WebSocketHandler& ws)
 inline void checkSessionLimitsAndWarnClients()
 {
 #ifndef MOBILEAPP
-    size_t docBrokerCount = DocBrokers.size() - ConvertToBroker::getInstanceCount();
-    if (docBrokerCount > LOOLWSD::MaxDocuments || LOOLWSD::NumConnections >= LOOLWSD::MaxConnections)
+    ssize_t docBrokerCount = DocBrokers.size() - ConvertToBroker::getInstanceCount();
+    if (LOOLWSD::MaxDocuments < 10000 &&
+        (docBrokerCount > LOOLWSD::MaxDocuments || LOOLWSD::NumConnections >= LOOLWSD::MaxConnections))
     {
         const std::string info = Poco::format(PAYLOAD_INFO_LIMIT_REACHED, LOOLWSD::MaxDocuments, LOOLWSD::MaxConnections);
         LOG_INF("Sending client 'limitreached' message: " << info);
@@ -278,7 +279,6 @@ inline void checkSessionLimitsAndWarnClients()
 /// connected to any document.
 void alertAllUsersInternal(const std::string& msg)
 {
-
     std::lock_guard<std::mutex> docBrokersLock(DocBrokersMutex);
 
     LOG_INF("Alerting all users: [" << msg << "]");
@@ -331,6 +331,7 @@ void cleanupDocBrokers()
         if (!docBroker->isAlive())
         {
             LOG_INF("Removing DocumentBroker for docKey [" << it->first << "].");
+            docBroker->dispose();
             it = DocBrokers.erase(it);
             continue;
         } else {