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

[Ashod Nakashian]

 loolwsd/LOOLWSD.cpp          |   46 ++++++++++++++++++++++++++-----------------
 loolwsd/test/httpwserror.cpp |    6 +++++
 2 files changed, 34 insertions(+), 18 deletions(-)

New commits:
commit f68ece00377ef41e68996f78f2ac4cd868794930
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Oct 23 11:51:10 2016 -0400

    loolwsd: fix testMaxDocuments unittest
    
    Checking for document limit must be done before allocating
    a child process, otherwise the new child process will not
    be cleaned up or released, thereby failing the test.
    
    Change-Id: I99b1155bdacf2f0b7a24c7b7330d207e4c7beee8
    Reviewed-on: https://gerrit.libreoffice.org/30208
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 6694f81..a9aa59d 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -192,17 +192,27 @@ void shutdownLimitReached(WebSocket& ws)
         int retries = 7;
         std::vector<char> buffer(READ_BUFFER_SIZE * 100);
 
-        // 5 seconds timeout
-        ws.setReceiveTimeout(5000000);
+        const Poco::Timespan waitTime(POLL_TIMEOUT_MS * 1000);
         do
         {
-            // ignore loolclient, load and partpagerectangles
-            ws.receiveFrame(buffer.data(), buffer.capacity(), flags);
-            if (--retries == 4)
+            if (ws.poll(Poco::Timespan(0), Poco::Net::Socket::SelectMode::SELECT_ERROR))
             {
-                ws.sendFrame(error.data(), error.size());
-                ws.shutdown(WebSocket::WS_POLICY_VIOLATION);
+                // Already disconnected, can't send 'close' frame.
+                ws.close();
+                return;
+            }
+
+            // Let the client know we are shutting down.
+            ws.sendFrame(error.data(), error.size());
+
+            // Ignore incoming messages.
+            if (ws.poll(waitTime, Poco::Net::Socket::SELECT_READ))
+            {
+                ws.receiveFrame(buffer.data(), buffer.capacity(), flags);
             }
+
+            // Shutdown.
+            ws.shutdown(WebSocket::WS_POLICY_VIOLATION);
         }
         while (retries > 0 && (flags & WebSocket::FRAME_OP_BITMASK) != WebSocket::FRAME_OP_CLOSE);
     }
@@ -684,6 +694,17 @@ private:
             }
             else
             {
+                // New Document.
+#if MAX_DOCUMENTS > 0
+                if (DocBrokers.size() + 1 > MAX_DOCUMENTS)
+                {
+                    Log::error() << "Limit on maximum number of open documents of "
+                                 << MAX_DOCUMENTS << " reached." << Log::end;
+                    shutdownLimitReached(*ws);
+                    return;
+                }
+#endif
+
                 // Store a dummy (marked to destroy) document broker until we
                 // have the real one, so that the other requests block
                 Log::debug("Inserting a dummy DocumentBroker for docKey [" + docKey + "] temporarily.");
@@ -759,17 +780,6 @@ private:
                 throw WebSocketErrorMessageException(SERVICE_UNAVAILABLE_INTERNAL_ERROR);
             }
 
-#if MAX_DOCUMENTS > 0
-            std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
-            if (DocBrokers.size() + 1 > MAX_DOCUMENTS)
-            {
-                Log::error("Maximum number of open documents reached.");
-                shutdownLimitReached(*ws);
-                return;
-            }
-            DocBrokersLock.unlock();
-#endif
-
             // Set one we just created.
             Log::debug("New DocumentBroker for docKey [" + docKey + "].");
             docBroker = std::make_shared<DocumentBroker>(uriPublic, docKey, LOOLWSD::ChildRoot, child);
diff --git a/loolwsd/test/httpwserror.cpp b/loolwsd/test/httpwserror.cpp
index 011806b..1d686e4 100644
--- a/loolwsd/test/httpwserror.cpp
+++ b/loolwsd/test/httpwserror.cpp
@@ -103,11 +103,15 @@ void HTTPWSError::testMaxDocuments()
         // Load a document.
         std::vector<std::shared_ptr<Poco::Net::WebSocket>> docs;
 
+        std::cerr << "Loading max number of documents: " << MAX_DOCUMENTS << std::endl;
         for (int it = 1; it <= MAX_DOCUMENTS; ++it)
         {
             docs.emplace_back(loadDocAndGetSocket("empty.odt", _uri, testname));
+            std::cerr << "Loaded document #" << it << " of " << MAX_DOCUMENTS << std::endl;
         }
 
+        std::cerr << "Loading one more document beyond the limit." << std::endl;
+
         // try to open MAX_DOCUMENTS + 1
         std::string docPath;
         std::string docURL;
@@ -121,6 +125,8 @@ void HTTPWSError::testMaxDocuments()
         sendTextFrame(socket, "load ", testname);
         sendTextFrame(socket, "partpagerectangles ", testname);
 
+        assertResponseString(socket, "error:", testname);
+
         std::string message;
         const auto statusCode = getErrorCode(socket, message);
         CPPUNIT_ASSERT_EQUAL(static_cast<int>(Poco::Net::WebSocket::WS_POLICY_VIOLATION), statusCode);