wsd/LOOLWSD.cpp | 76 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 34 deletions(-)
New commits: commit b892d0c41f12a6866b6aefa1716a37dd1f742361 Author: Michael Meeks <[email protected]> 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); _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
