[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3' - 5 commits - wsd/ClientSession.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

Michael Meeks (via logerrit) logerrit at kemper.freedesktop.org
Fri Jun 7 20:41:23 UTC 2019


 wsd/ClientSession.cpp  |   14 +++++++++++---
 wsd/DocumentBroker.cpp |   38 +++++++++++++++++++++++++++++++-------
 wsd/DocumentBroker.hpp |   11 +++++++----
 wsd/LOOLWSD.cpp        |   13 ++++++++++++-
 4 files changed, 61 insertions(+), 15 deletions(-)

New commits:
commit 61d5a3d9a06d0e7a0dcc7ac868c728a9c01a2952
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Wed May 22 02:38:39 2019 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Fri Jun 7 21:26:49 2019 +0100

    Avoid exceptions in some shutdown corner cases.
    
    Change-Id: I1c301dc96d925fd5d74c00bf4b9417782822a997

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 32dbd37ca..0c5ad5020 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1850,11 +1850,14 @@ void ConvertToBroker::removeFile(const std::string &uriOrig)
 {
     if (!uriOrig.empty())
     {
-        // Remove source file and directory
-        Poco::Path path = uriOrig;
-        Poco::File(path).remove();
-        Poco::File(path.makeParent()).remove();
-        FileUtil::removeFile(uriOrig);
+        try {
+            // Remove source file and directory
+            Poco::Path path = uriOrig;
+            Poco::File(path).remove();
+            Poco::File(path.makeParent()).remove();
+        } catch (const std::exception &ex) {
+            LOG_ERR("Error while removing conversion temporary: '" << uriOrig << "' - " << ex.what());
+        }
     }
 }
 
commit db96450f370f0edd1d64af3e60272b3caa8c503d
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Tue May 21 19:50:17 2019 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Fri Jun 7 21:26:33 2019 +0100

    tdf#123482 - cleanup convert-to folder even more reliably.
    
    Problems could occur if exceptiosn thrown when parsing the input stream.
    
    Change-Id: Id82b3816450194164fc2093554c730b4a94acef1
    Reviewed-on: https://gerrit.libreoffice.org/72695
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 7623c8ab3..32dbd37ca 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1843,13 +1843,18 @@ ConvertToBroker::ConvertToBroker(const std::string& uri,
 ConvertToBroker::~ConvertToBroker()
 {
     NumConverters--;
-    if (!_uriOrig.empty())
+    removeFile(_uriOrig);
+}
+
+void ConvertToBroker::removeFile(const std::string &uriOrig)
+{
+    if (!uriOrig.empty())
     {
         // Remove source file and directory
-        Poco::Path path = _uriOrig;
+        Poco::Path path = uriOrig;
         Poco::File(path).remove();
         Poco::File(path.makeParent()).remove();
-        FileUtil::removeFile(_uriOrig);
+        FileUtil::removeFile(uriOrig);
     }
 }
 
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index adc98ee7f..fe51d7194 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -474,6 +474,9 @@ public:
 
     /// How many live conversions are running.
     static size_t getInstanceCount();
+
+    /// Cleanup path and its parent
+    static void removeFile(const std::string &uri);
 };
 
 #endif
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 2ab3b4af8..996f974e2 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -503,6 +503,9 @@ class ConvertToPartHandler : public PartHandler
 public:
     std::string getFilename() const { return _filename; }
 
+    /// Afterwards someone else is responsible for cleaning that up.
+    void takeFile() { _filename.clear(); }
+
     ConvertToPartHandler(bool convertTo = false)
         : _convertTo(convertTo)
     {
@@ -510,6 +513,11 @@ public:
 
     virtual ~ConvertToPartHandler()
     {
+        if (!_filename.empty())
+        {
+            LOG_TRC("Remove un-handled temporary file '" << _filename << "'");
+            ConvertToBroker::removeFile(_filename);
+        }
     }
 
     virtual void handlePart(const MessageHeader& header, std::istream& stream) override
@@ -2235,6 +2243,7 @@ private:
 
                     LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
                     auto docBroker = std::make_shared<ConvertToBroker>(fromPath, uriPublic, docKey);
+                    handler.takeFile();
 
                     cleanupDocBrokers();
 
commit 136622a82d5e547cf8836011051743d4b73d5020
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Tue Mar 12 15:41:54 2019 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Fri Jun 7 21:25:05 2019 +0100

    Don't count convert-to connections vs. the document count.
    
    Change-Id: I350905fb98c503ae8f22a377e4af5cbcb9f3c52d
    Reviewed-on: https://gerrit.libreoffice.org/71724
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 465549c27..7623c8ab3 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1825,8 +1825,24 @@ void DocumentBroker::getIOStats(uint64_t &sent, uint64_t &recv)
     }
 }
 
+static std::atomic<size_t> NumConverters;
+
+size_t ConvertToBroker::getInstanceCount()
+{
+    return NumConverters;
+}
+
+ConvertToBroker::ConvertToBroker(const std::string& uri,
+                                 const Poco::URI& uriPublic,
+                                 const std::string& docKey)
+    : DocumentBroker(uri, uriPublic, docKey)
+{
+    NumConverters++;
+}
+
 ConvertToBroker::~ConvertToBroker()
 {
+    NumConverters--;
     if (!_uriOrig.empty())
     {
         // Remove source file and directory
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 8a284f965..adc98ee7f 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -469,11 +469,11 @@ public:
     /// Construct DocumentBroker with URI and docKey
     ConvertToBroker(const std::string& uri,
                     const Poco::URI& uriPublic,
-                    const std::string& docKey)
-        : DocumentBroker(uri, uriPublic, docKey)
-    {
-    }
+                    const std::string& docKey);
     virtual ~ConvertToBroker();
+
+    /// How many live conversions are running.
+    static size_t getInstanceCount();
 };
 
 #endif
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index e692e3735..2ab3b4af8 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -225,7 +225,8 @@ inline void shutdownLimitReached(WebSocketHandler& ws)
 
 inline void checkSessionLimitsAndWarnClients()
 {
-    if (DocBrokers.size() > LOOLWSD::MaxDocuments || LOOLWSD::NumConnections >= LOOLWSD::MaxConnections)
+    size_t docBrokerCount = DocBrokers.size() - ConvertToBroker::getInstanceCount();
+    if (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);
@@ -2779,6 +2780,7 @@ public:
                   << "[ " << DocBrokers.size() << " ]:\n";
         for (auto &i : DocBrokers)
             i.second->dumpState(os);
+        os << "Converter count: " << ConvertToBroker::getInstanceCount() << "\n";
 
         Socket::InhibitThreadChecks = false;
         SocketPoll::InhibitThreadChecks = false;
commit 08d5c46e2eb9b17c425f3343b8cffc8cc65889e0
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Fri May 17 14:26:07 2019 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Thu May 23 17:59:38 2019 +0100

    Tolerate empty first lines.
    
    Change-Id: Ib9aaf82560fc3f5adaa97f40a3de5f3946c6f65d

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 6e72c6fdf..356d86e1c 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -85,6 +85,12 @@ bool ClientSession::_handleInput(const char *buffer, int length)
         return false;
     }
 
+    if (tokens.size() < 1)
+    {
+        sendTextFrame("error: cmd=empty kind=unknown");
+        return false;
+    }
+
     LOOLWSD::dumpIncomingTrace(docBroker->getJailId(), getId(), firstLine);
 
     if (LOOLProtocol::tokenIndicatesUserInteraction(tokens[0]))
@@ -95,7 +101,7 @@ bool ClientSession::_handleInput(const char *buffer, int length)
     }
     if (tokens[0] == "loolclient")
     {
-        if (tokens.size() < 1)
+        if (tokens.size() < 2)
         {
             sendTextFrame("error: cmd=loolclient kind=badprotocolversion");
             return false;
commit 4389a4cdcfdfd8c3ca2b0aacd6681033a9dff08f
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Thu May 23 17:55:01 2019 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Thu May 23 17:57:57 2019 +0100

    Make renamefile more careful.
    
    Change-Id: If39353fc01ea48d8e0077b228a6281839dde5c87

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index cc325b44b..6e72c6fdf 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -208,8 +208,10 @@ bool ClientSession::_handleInput(const char *buffer, int length)
 
         return true;
     }
-    else if (tokens[0] == "versionrestore") {
-        if (tokens[1] == "prerestore") {
+    else if (tokens[0] == "versionrestore")
+    {
+        if (tokens.size() > 1 && tokens[1] == "prerestore")
+        {
             // green signal to WOPI host to restore the version *after* saving
             // any unsaved changes, if any, to the storage
             docBroker->closeDocument("versionrestore: prerestore_ack");


More information about the Libreoffice-commits mailing list