From 59acd6de9032bf42edff7f542f52e1a8e39ef1d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= Date: Wed, 20 Nov 2024 15:29:45 +0000 Subject: [PATCH 1/4] last arg to findOrCreateDocBroker is always null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Caolán McNamara Change-Id: I9f542a0d4093a102c09d77f3be3cccb635d7a721 --- wsd/ClientRequestDispatcher.cpp | 7 +++---- wsd/RequestVettingStation.cpp | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/wsd/ClientRequestDispatcher.cpp b/wsd/ClientRequestDispatcher.cpp index ae572a624a1bb..435e84809c1bd 100644 --- a/wsd/ClientRequestDispatcher.cpp +++ b/wsd/ClientRequestDispatcher.cpp @@ -110,8 +110,7 @@ inline void shutdownLimitReached(const std::shared_ptr std::pair, std::string> findOrCreateDocBroker(DocumentBroker::ChildType type, const std::string& uri, const std::string& docKey, const std::string& id, const Poco::URI& uriPublic, - unsigned mobileAppDocId, - std::unique_ptr wopiFileInfo) + unsigned mobileAppDocId) { LOG_INF("Find or create DocBroker for docKey [" << docKey << "] for session [" << id << "] on url [" @@ -184,7 +183,7 @@ findOrCreateDocBroker(DocumentBroker::ChildType type, const std::string& uri, // Set the one we just created. LOG_DBG("New DocumentBroker for docKey [" << docKey << ']'); docBroker = std::make_shared(type, uri, uriPublic, docKey, mobileAppDocId, - std::move(wopiFileInfo)); + nullptr); DocBrokers.emplace(docKey, docBroker); LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << ']'); } @@ -1838,7 +1837,7 @@ bool ClientRequestDispatcher::handleClientProxyRequest(const Poco::Net::HTTPRequ // Request a kit process for this doc. std::pair, std::string> pair = findOrCreateDocBroker(DocumentBroker::ChildType::Interactive, url, docKey, _id, uriPublic, - /*mobileAppDocId=*/0, /*wopiFileInfo=*/nullptr); + /*mobileAppDocId=*/0); auto docBroker = pair.first; if (!docBroker) diff --git a/wsd/RequestVettingStation.cpp b/wsd/RequestVettingStation.cpp index 56c9bd14b0173..c77a7d01ff6ca 100644 --- a/wsd/RequestVettingStation.cpp +++ b/wsd/RequestVettingStation.cpp @@ -33,8 +33,7 @@ extern std::pair, std::string> findOrCreateDocBroker(DocumentBroker::ChildType type, const std::string& uri, const std::string& docKey, const std::string& id, const Poco::URI& uriPublic, - unsigned mobileAppDocId, - std::unique_ptr wopiFileInfo); + unsigned mobileAppDocId); namespace { @@ -341,7 +340,7 @@ bool RequestVettingStation::createDocBroker(const std::string& docKey, const std // Request a kit process for this doc. const auto [docBroker, error] = findOrCreateDocBroker(DocumentBroker::ChildType::Interactive, url, docKey, _id, uriPublic, - _mobileAppDocId, /*wopiFileInfo=*/nullptr); + _mobileAppDocId); _docBroker = docBroker; if (_docBroker) From 9a8f66b1d5c8e2ed2c803c29a2f8491ee9ea6650 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= Date: Wed, 20 Nov 2024 15:34:10 +0000 Subject: [PATCH 2/4] wopiFileInfo arg to DocumentBroker is always null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Caolán McNamara Change-Id: I813540cc7e2524e862b559bae0bd11fc69b63f39 --- wsd/ClientRequestDispatcher.cpp | 3 +-- wsd/DocumentBroker.cpp | 10 +--------- wsd/DocumentBroker.hpp | 6 ++---- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/wsd/ClientRequestDispatcher.cpp b/wsd/ClientRequestDispatcher.cpp index 435e84809c1bd..ed83210f5954a 100644 --- a/wsd/ClientRequestDispatcher.cpp +++ b/wsd/ClientRequestDispatcher.cpp @@ -182,8 +182,7 @@ findOrCreateDocBroker(DocumentBroker::ChildType type, const std::string& uri, // Set the one we just created. LOG_DBG("New DocumentBroker for docKey [" << docKey << ']'); - docBroker = std::make_shared(type, uri, uriPublic, docKey, mobileAppDocId, - nullptr); + docBroker = std::make_shared(type, uri, uriPublic, docKey, mobileAppDocId); DocBrokers.emplace(docKey, docBroker); LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << ']'); } diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 26cb2a4083af3..dfbad935de584 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -158,8 +158,7 @@ class DocumentBroker::DocumentBrokerPoll final : public TerminatingPoll std::atomic DocumentBroker::DocBrokerId(1); DocumentBroker::DocumentBroker(ChildType type, const std::string& uri, const Poco::URI& uriPublic, - const std::string& docKey, unsigned mobileAppDocId, - std::unique_ptr wopiFileInfo) + const std::string& docKey, unsigned mobileAppDocId) : _limitLifeSeconds(std::chrono::seconds::zero()) , _uriOrig(uri) , _type(type) @@ -221,13 +220,6 @@ DocumentBroker::DocumentBroker(ChildType type, const std::string& uri, const Poc { _unitWsd->onDocBrokerCreate(_docKey); } - - _initialWopiFileInfo = std::move(wopiFileInfo); - if (_initialWopiFileInfo) - { - LOG_DBG("Starting DocBrokerPoll thread"); - _poll->startThread(); - } } pid_t DocumentBroker::getPid() const { return _childProcess ? _childProcess->getPid() : 0; } diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 53c9c00ae8a45..e65cbc9a39b5d 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -232,15 +232,13 @@ class DocumentBroker : public std::enable_shared_from_this }; DocumentBroker(ChildType type, const std::string& uri, const Poco::URI& uriPublic, - const std::string& docKey, unsigned mobileAppDocId, - std::unique_ptr wopiFileInfo); + const std::string& docKey, unsigned mobileAppDocId); protected: /// Used by derived classes. DocumentBroker(ChildType type, const std::string& uri, const Poco::URI& uriPublic, const std::string& docKey) - : DocumentBroker(type, uri, uriPublic, docKey, /*mobileAppDocId=*/0, - /*wopiFileInfo=*/nullptr) + : DocumentBroker(type, uri, uriPublic, docKey, /*mobileAppDocId=*/0) { } From d181e89e5b76f7dec60ab4de2b84a79490a1b0fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= Date: Wed, 20 Nov 2024 15:35:10 +0000 Subject: [PATCH 3/4] _initialWopiFileInfo is never set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Caolán McNamara Change-Id: I5c90b48629c52cf4db76a174c15470d9e48294d4 --- wsd/DocumentBroker.cpp | 24 ------------------------ wsd/DocumentBroker.hpp | 4 ---- 2 files changed, 28 deletions(-) diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index dfbad935de584..b122064f43eb9 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -314,30 +314,6 @@ void DocumentBroker::pollThread() setupPriorities(); - // Download and load the document. - if (_initialWopiFileInfo) - { - try - { - downloadAdvance(_childProcess->getJailId(), _uriPublic, std::move(_initialWopiFileInfo)); - } - catch (const std::exception& exc) - { - LOG_ERR("Failed to advance download [" << _docKey << "]: " << exc.what()); - - stop("advance download failed"); - - // Stop to mark it done and cleanup. - _poll->stop(); - - // Async cleanup. - COOLWSD::doHousekeeping(); - - return; - } - } - - #if !MOBILEAPP CONFIG_STATIC const std::size_t IdleDocTimeoutSecs = ConfigUtil::getConfigValue("per_document.idle_timeout_secs", 3600); diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index e65cbc9a39b5d..8e41417c1b041 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -1406,10 +1406,6 @@ class DocumentBroker : public std::enable_shared_from_this std::string _filename; std::atomic _migrateMsgReceived = false; - /// The WopiFileInfo of the initial request loading the document for the first time. - /// This has a single-use, and then it's reset. - std::unique_ptr _initialWopiFileInfo; - /// The state of the document. /// This regulates all other primary operations. class DocumentState final From 51cdc811c5785407cf68b4358b280230a706f710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= Date: Wed, 20 Nov 2024 15:35:52 +0000 Subject: [PATCH 4/4] downloadAdvance is unused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Caolán McNamara Change-Id: I288872f5dc7a4680dc88c7bb2b6dea757bdaf2c2 --- wsd/DocumentBroker.cpp | 15 --------------- wsd/DocumentBroker.hpp | 4 ---- 2 files changed, 19 deletions(-) diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index b122064f43eb9..85e58a6fd79c0 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -888,21 +888,6 @@ void DocumentBroker::stop(const std::string& reason) _poll->wakeup(); } -bool DocumentBroker::downloadAdvance(const std::string& jailId, const Poco::URI& uriPublic, - std::unique_ptr wopiFileInfo) -{ - ASSERT_CORRECT_THREAD(); - - LOG_INF("Loading [" << _docKey << "] ahead-of-time in jail [" << jailId << ']'); - - assert(!_docState.isMarkedToDestroy() && "MarkedToDestroy while downloading ahead-of-time"); - - assert(_storage == nullptr && - "Unexpected to find storage created while downloading ahead-of-time"); - - return download(/*session=*/nullptr, jailId, uriPublic, std::move(wopiFileInfo)); -} - bool DocumentBroker::download( const std::shared_ptr& session, const std::string& jailId, const Poco::URI& uriPublic, diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 8e41417c1b041..a0e20dd6bbc9f 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -547,10 +547,6 @@ class DocumentBroker : public std::enable_shared_from_this void refreshLock(); - /// Downloads the document ahead-of-time. - bool downloadAdvance(const std::string& jailId, const Poco::URI& uriPublic, - std::unique_ptr wopiFileInfo); - /// Loads a document from the public URI into the jail. bool download(const std::shared_ptr& session, const std::string& jailId, const Poco::URI& uriPublic,