Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

last arg to findOrCreateDocBroker is always null #10552

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

last arg to findOrCreateDocBroker is always null #10552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
59acd6d
last arg to findOrCreateDocBroker is always null
caolanm Nov 20, 2024
9a8f66b
wopiFileInfo arg to DocumentBroker is always null
caolanm Nov 20, 2024
d181e89
_initialWopiFileInfo is never set
caolanm Nov 20, 2024
51cdc81
downloadAdvance is unused
caolanm Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions wsd/ClientRequestDispatcher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ inline void shutdownLimitReached(const std::shared_ptr<ProtocolHandlerInterface>
std::pair<std::shared_ptr<DocumentBroker>, 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<WopiStorage::WOPIFileInfo> wopiFileInfo)
unsigned mobileAppDocId)
{
LOG_INF("Find or create DocBroker for docKey ["
<< docKey << "] for session [" << id << "] on url ["
Expand Down Expand Up @@ -183,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<DocumentBroker>(type, uri, uriPublic, docKey, mobileAppDocId,
std::move(wopiFileInfo));
docBroker = std::make_shared<DocumentBroker>(type, uri, uriPublic, docKey, mobileAppDocId);
DocBrokers.emplace(docKey, docBroker);
LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after inserting [" << docKey << ']');
}
Expand Down Expand Up @@ -1838,7 +1836,7 @@ bool ClientRequestDispatcher::handleClientProxyRequest(const Poco::Net::HTTPRequ
// Request a kit process for this doc.
std::pair<std::shared_ptr<DocumentBroker>, std::string> pair
= findOrCreateDocBroker(DocumentBroker::ChildType::Interactive, url, docKey, _id, uriPublic,
/*mobileAppDocId=*/0, /*wopiFileInfo=*/nullptr);
/*mobileAppDocId=*/0);
auto docBroker = pair.first;

if (!docBroker)
Expand Down
49 changes: 1 addition & 48 deletions wsd/DocumentBroker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ class DocumentBroker::DocumentBrokerPoll final : public TerminatingPoll
std::atomic<unsigned> DocumentBroker::DocBrokerId(1);

DocumentBroker::DocumentBroker(ChildType type, const std::string& uri, const Poco::URI& uriPublic,
const std::string& docKey, unsigned mobileAppDocId,
std::unique_ptr<WopiStorage::WOPIFileInfo> wopiFileInfo)
const std::string& docKey, unsigned mobileAppDocId)
: _limitLifeSeconds(std::chrono::seconds::zero())
, _uriOrig(uri)
, _type(type)
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -322,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<int>("per_document.idle_timeout_secs", 3600);
Expand Down Expand Up @@ -920,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<WopiStorage::WOPIFileInfo> 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<ClientSession>& session, const std::string& jailId,
const Poco::URI& uriPublic,
Expand Down
14 changes: 2 additions & 12 deletions wsd/DocumentBroker.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,13 @@ class DocumentBroker : public std::enable_shared_from_this<DocumentBroker>
};

DocumentBroker(ChildType type, const std::string& uri, const Poco::URI& uriPublic,
const std::string& docKey, unsigned mobileAppDocId,
std::unique_ptr<WopiStorage::WOPIFileInfo> 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)
{
}

Expand Down Expand Up @@ -549,10 +547,6 @@ class DocumentBroker : public std::enable_shared_from_this<DocumentBroker>

void refreshLock();

/// Downloads the document ahead-of-time.
bool downloadAdvance(const std::string& jailId, const Poco::URI& uriPublic,
std::unique_ptr<WopiStorage::WOPIFileInfo> wopiFileInfo);

/// Loads a document from the public URI into the jail.
bool download(const std::shared_ptr<ClientSession>& session, const std::string& jailId,
const Poco::URI& uriPublic,
Expand Down Expand Up @@ -1408,10 +1402,6 @@ class DocumentBroker : public std::enable_shared_from_this<DocumentBroker>
std::string _filename;
std::atomic<bool> _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<WopiStorage::WOPIFileInfo> _initialWopiFileInfo;

/// The state of the document.
/// This regulates all other primary operations.
class DocumentState final
Expand Down
5 changes: 2 additions & 3 deletions wsd/RequestVettingStation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
extern std::pair<std::shared_ptr<DocumentBroker>, 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<WopiStorage::WOPIFileInfo> wopiFileInfo);
unsigned mobileAppDocId);

namespace
{
Expand Down Expand Up @@ -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)
Expand Down
Loading