[Libreoffice-commits] online.git: wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

[Michael Meeks (via logerrit)]

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

New commits:
commit 5e7cac28fa74131461f6d9e25334e9cc7568f444
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 11:01:45 2019 +0100

    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

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index ab71fe5ba..c332f7c52 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -2072,10 +2072,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 576906d9e..7c66832a5 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();
 
@@ -420,7 +423,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.
@@ -498,6 +501,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 f404798d0..b460d75d8 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -258,8 +258,9 @@ inline void shutdownLimitReached(WebSocketHandler& ws)
 inline void checkSessionLimitsAndWarnClients()
 {
 #if !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);
@@ -333,6 +334,7 @@ void cleanupDocBrokers()
         if (!docBroker->isAlive())
         {
             LOG_INF("Removing DocumentBroker for docKey [" << it->first << "].");
+            docBroker->dispose();
             it = DocBrokers.erase(it);
             continue;
         } else {