[Libreoffice-commits] online.git: Branch 'private/Ashod/nonblocking' - wsd/LOOLWSD.cpp

Michael Meeks michael.meeks at collabora.com
Sat Mar 4 21:37:52 UTC 2017


 wsd/LOOLWSD.cpp |   76 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 42 insertions(+), 34 deletions(-)

New commits:
commit b892d0c41f12a6866b6aefa1716a37dd1f742361
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Sat Mar 4 21:37:10 2017 +0000

    Annotate some blocking methods - ugly, but necessary.

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 8fdf280..ce39f25 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -463,7 +463,7 @@ static size_t addNewChild(const std::shared_ptr<ChildProcess>& child)
     return count;
 }
 
-static std::shared_ptr<ChildProcess> getNewChild()
+static std::shared_ptr<ChildProcess> getNewChild_Blocks()
 {
     Util::assertIsLocked(DocBrokersMutex);
     std::unique_lock<std::mutex> lock(NewChildrenMutex);
@@ -490,6 +490,7 @@ static std::shared_ptr<ChildProcess> getNewChild()
 #endif
         LOG_TRC("Waiting for a new child for a max of " << timeoutMs << " ms.");
         const auto timeout = chrono::milliseconds(timeoutMs);
+        // FIXME: blocks ...
         if (NewChildrenCV.wait_for(lock, timeout, []() { return !NewChildren.empty(); }))
         {
             auto child = NewChildren.back();
@@ -589,7 +590,7 @@ private:
     /// Handle POST requests.
     /// Always throw on error, do not set response status here.
     /// Returns true if a response has been sent.
-    static bool handlePostRequest(HTTPServerRequest& request, HTTPServerResponse& response, const std::string& id)
+    static bool handlePostRequest_Blocks(HTTPServerRequest& request, HTTPServerResponse& response, const std::string& id)
     {
         LOG_INF("Post request: [" << request.getURI() << "]");
         StringTokenizer tokens(request.getURI(), "/?");
@@ -615,7 +616,7 @@ private:
                     std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
 
                     // Request a kit process for this doc.
-                    auto child = getNewChild();
+                    auto child = getNewChild_Blocks();
                     if (!child)
                     {
                         // Let the client know we can't serve now.
@@ -827,7 +828,7 @@ private:
     }
 
     /// Handle GET requests.
-    static void handleGetRequest(const std::string& uri, std::shared_ptr<LOOLWebSocket>& ws, const std::string& id)
+    static void handleGetRequest_Blocks(const std::string& uri, std::shared_ptr<LOOLWebSocket>& ws, const std::string& id)
     {
         LOG_INF("Starting GET request handler for session [" << id << "] on url [" << uri << "].");
         try
@@ -864,7 +865,7 @@ private:
             int retry = 3;
             while (retry-- > 0)
             {
-                auto docBroker = findOrCreateDocBroker(uri, docKey, ws, id, uriPublic);
+                auto docBroker = findOrCreateDocBroker_Blocks(uri, docKey, ws, id, uriPublic);
                 if (docBroker)
                 {
                     auto session = createNewClientSession(ws, id, uriPublic, docBroker, isReadOnly);
@@ -913,11 +914,11 @@ private:
     /// Otherwise, creates and adds a new one to DocBrokers.
     /// May return null if terminating or MaxDocuments limit is reached.
     /// After returning a valid instance DocBrokers must be cleaned up after exceptions.
-    static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(const std::string& uri,
-                                                                 const std::string& docKey,
-                                                                 std::shared_ptr<LOOLWebSocket>& ws,
-                                                                 const std::string& id,
-                                                                 const Poco::URI& uriPublic)
+    static std::shared_ptr<DocumentBroker> findOrCreateDocBroker_Blocks(const std::string& uri,
+                                                                        const std::string& docKey,
+                                                                        std::shared_ptr<LOOLWebSocket>& ws,
+                                                                        const std::string& id,
+                                                                        const Poco::URI& uriPublic)
     {
         LOG_INF("Find or create DocBroker for docKey [" << docKey <<
                 "] for session [" << id << "] on url [" << uriPublic.toString() << "].");
@@ -1025,10 +1026,10 @@ private:
         return docBroker;
     }
 
-    static std::shared_ptr<DocumentBroker> createNewDocBroker(const std::string& uri,
-                                                              const std::string& docKey,
-                                                              std::shared_ptr<LOOLWebSocket>& ws,
-                                                              const Poco::URI& uriPublic)
+    static std::shared_ptr<DocumentBroker> createNewDocBroker_Blocks(const std::string& uri,
+                                                                     const std::string& docKey,
+                                                                     std::shared_ptr<LOOLWebSocket>& ws,
+                                                                     const Poco::URI& uriPublic)
     {
         Util::assertIsLocked(DocBrokersMutex);
 
@@ -1041,7 +1042,8 @@ private:
         }
 
         // Request a kit process for this doc.
-        auto child = getNewChild();
+        // FIXME: Blocks !
+        auto child = getNewChild_Blocks();
         if (!child)
         {
             // Let the client know we can't serve now.
@@ -1368,13 +1370,13 @@ public:
                      reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
             {
                 // All post requests have url prefix 'lool'.
-                responded = handlePostRequest(request, response, id);
+                responded = handlePostRequest_Blocks(request, response, id);
             }
             else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws")
             {
                 auto ws = std::make_shared<LOOLWebSocket>(request, response);
                 responded = true; // After upgrading to WS we should not set HTTP response.
-                handleGetRequest(reqPathTokens[1], ws, id);
+                handleGetRequest_Blocks(reqPathTokens[1], ws, id);
             }
             else
             {
@@ -2342,10 +2344,10 @@ bool LOOLWSD::createForKit()
 std::mutex Connection::Mutex;
 #endif
 
-static std::shared_ptr<DocumentBroker> createDocBroker(WebSocketHandler& ws,
-                                                       const std::string& uri,
-                                                       const std::string& docKey,
-                                                       const Poco::URI& uriPublic)
+static std::shared_ptr<DocumentBroker> createDocBroker_Blocks(WebSocketHandler& ws,
+                                                              const std::string& uri,
+                                                              const std::string& docKey,
+                                                              const Poco::URI& uriPublic)
 {
     Util::assertIsLocked(DocBrokersMutex);
 
@@ -2358,7 +2360,9 @@ static std::shared_ptr<DocumentBroker> createDocBroker(WebSocketHandler& ws,
     }
 
     // Request a kit process for this doc.
-    auto child = getNewChild();
+
+    // FIXME: blocks !
+    auto child = getNewChild_Blocks();
     if (!child)
     {
         // Let the client know we can't serve now.
@@ -2380,11 +2384,11 @@ static std::shared_ptr<DocumentBroker> createDocBroker(WebSocketHandler& ws,
 /// Otherwise, creates and adds a new one to DocBrokers.
 /// May return null if terminating or MaxDocuments limit is reached.
 /// After returning a valid instance DocBrokers must be cleaned up after exceptions.
-static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& ws,
-                                                             const std::string& uri,
-                                                             const std::string& docKey,
-                                                             const std::string& id,
-                                                             const Poco::URI& uriPublic)
+static std::shared_ptr<DocumentBroker> findOrCreateDocBroker_Blocks(WebSocketHandler& ws,
+                                                                    const std::string& uri,
+                                                                    const std::string& docKey,
+                                                                    const std::string& id,
+                                                                    const Poco::URI& uriPublic)
 {
     LOG_INF("Find or create DocBroker for docKey [" << docKey <<
             "] for session [" << id << "] on url [" << uriPublic.toString() << "].");
@@ -2419,6 +2423,8 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w
             bool timedOut = true;
             for (size_t i = 0; i < COMMAND_TIMEOUT_MS / POLL_TIMEOUT_MS; ++i)
             {
+
+                // FIXME: blocks !
                 std::this_thread::sleep_for(std::chrono::milliseconds(POLL_TIMEOUT_MS));
 
                 docBrokersLock.lock();
@@ -2486,7 +2492,7 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w
 
     if (!docBroker)
     {
-        docBroker = createDocBroker(ws, uri, docKey, uriPublic);
+        docBroker = createDocBroker_Blocks(ws, uri, docKey, uriPublic);
     }
 
     return docBroker;
@@ -2715,11 +2721,11 @@ private:
                     reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
                 {
                     // All post requests have url prefix 'lool'.
-                    handlePostRequest(request, message);
+                    handlePostRequest_Blocks(request, message);
                 }
                 else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws")
                 {
-                    handleClientWsRequest(request, reqPathTokens[1]);
+                    handleClientWsRequest_Blocks(request, reqPathTokens[1]);
                 }
                 else
                 {
@@ -2874,7 +2880,7 @@ private:
         return "application/octet-stream";
     }
 
-    void handlePostRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
+    void handlePostRequest_Blocks(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
     {
         LOG_INF("Post request: [" << request.getURI() << "]");
 
@@ -2904,7 +2910,7 @@ private:
                     std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
 
                     // Request a kit process for this doc.
-                    auto child = getNewChild();
+                    auto child = getNewChild_Blocks();
                     if (!child)
                     {
                         // Let the client know we can't serve now.
@@ -3120,7 +3126,7 @@ private:
         throw BadRequestException("Invalid or unknown request.");
     }
 
-    void handleClientWsRequest(const Poco::Net::HTTPRequest& request, const std::string& url)
+    void handleClientWsRequest_Blocks(const Poco::Net::HTTPRequest& request, const std::string& url)
     {
         // requestHandler = new ClientRequestHandler();
         LOG_INF("Client WS request" << request.getURI() << ", url: " << url);
@@ -3160,11 +3166,13 @@ private:
 
         LOG_INF("URL [" << url << "] is " << (isReadOnly ? "readonly" : "writable") << ".");
 
+        // FIXME: we need to push this all out into its own thread - to not block.
+
         // Request a kit process for this doc.
         int retry = 3;
         while (retry-- > 0)
         {
-            auto docBroker = findOrCreateDocBroker(ws, url, docKey, _id, uriPublic);
+            auto docBroker = findOrCreateDocBroker_Blocks(ws, url, docKey, _id, uriPublic);
             if (docBroker)
             {
                 _clientSession = createNewClientSession(ws, _id, uriPublic, docBroker, isReadOnly);


More information about the Libreoffice-commits mailing list