loolwsd/ChildProcessSession.cpp | 1 loolwsd/DocumentBroker.cpp | 97 +++++++++++++++++++++------------------- loolwsd/DocumentBroker.hpp | 26 +++++++--- loolwsd/LOOLWSD.cpp | 7 -- 4 files changed, 71 insertions(+), 60 deletions(-)
New commits: commit d07a5824695178fe2a8493131c1a39e07a2c7c24 Author: Ashod Nakashian <[email protected]> Date: Mon Apr 25 20:48:02 2016 -0400 loolwsd: merged autoSave and waitSave This makes for amore compact API and avoids a race between issuing the save and waiting for it. Also added force flag and autoSave now checks the modified state of the document. If a document is not modified, nor save forced, autoSave checks the last activity on the document and only if there is any since last save does it issue a save command. Change-Id: I962e36df18d7edf5f658992e97b5def5f6247dc3 Reviewed-on: https://gerrit.libreoffice.org/24382 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/ChildProcessSession.cpp b/loolwsd/ChildProcessSession.cpp index 406a4a3..0def91d 100644 --- a/loolwsd/ChildProcessSession.cpp +++ b/loolwsd/ChildProcessSession.cpp @@ -314,7 +314,6 @@ void ChildProcessSession::disconnect() if (_multiView) _loKitDocument->pClass->setView(_loKitDocument, _viewId); - //TODO: Handle saving to temp etc. _onUnload(getId()); LOOLSession::disconnect(); diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp index e7fb11f..62485ae 100644 --- a/loolwsd/DocumentBroker.cpp +++ b/loolwsd/DocumentBroker.cpp @@ -180,8 +180,10 @@ bool DocumentBroker::save() return false; } -bool DocumentBroker::autoSave(const bool force) +bool DocumentBroker::autoSave(const bool force, const size_t waitTimeoutMs) { + Log::trace("Autosaving [" + _docKey + "]."); + std::unique_lock<std::mutex> lock(_mutex); if (_sessions.empty()) { @@ -189,69 +191,74 @@ bool DocumentBroker::autoSave(const bool force) return false; } - // Find the most recent activity. - double inactivityTimeMs = std::numeric_limits<double>::max(); - for (auto& sessionIt: _sessions) + bool sent = false; + if (force || _isModified) { - inactivityTimeMs = std::min(sessionIt.second->getInactivityMS(), inactivityTimeMs); + sent = sendUnoSave(); } - - Log::trace("Most recent activity was " + std::to_string((int)inactivityTimeMs) + " ms ago."); - const auto timeSinceLastSaveMs = getTimeSinceLastSaveMs(); - Log::trace("Time since last save is " + std::to_string((int)timeSinceLastSaveMs) + " ms."); - - // There has been some editing since we saved last? - if (inactivityTimeMs < timeSinceLastSaveMs) + else { - // Either we've been idle long enough, or it's auto-save time. - // Or we are asked to save anyway. - if (inactivityTimeMs >= IdleSaveDurationMs || - timeSinceLastSaveMs >= AutoSaveDurationMs || - force) + // Find the most recent activity. + double inactivityTimeMs = std::numeric_limits<double>::max(); + for (auto& sessionIt: _sessions) { - Log::info("Auto-save triggered for doc [" + _docKey + "]."); + inactivityTimeMs = std::min(sessionIt.second->getInactivityMS(), inactivityTimeMs); + } - // Save using session holding the edit-lock - bool sent = false; - for (auto& sessionIt: _sessions) - { - if (!sessionIt.second->isEditLocked()) - continue; - - auto queue = sessionIt.second->getQueue(); - if (queue) - { - queue->put("uno .uno:Save"); - sent = true; - break; - } - } + Log::trace("Most recent activity was " + std::to_string((int)inactivityTimeMs) + " ms ago."); + const auto timeSinceLastSaveMs = getTimeSinceLastSaveMs(); + Log::trace("Time since last save is " + std::to_string((int)timeSinceLastSaveMs) + " ms."); - if (!sent) + // There has been some editing since we saved last? + if (inactivityTimeMs < timeSinceLastSaveMs) + { + // Either we've been idle long enough, or it's auto-save time. + // Or we are asked to save anyway. + if (inactivityTimeMs >= IdleSaveDurationMs || + timeSinceLastSaveMs >= AutoSaveDurationMs) { - Log::error("Failed to auto-save doc [" + _docKey + "]: No valid sessions."); + sent = sendUnoSave(); } + } + } + + if (sent && waitTimeoutMs > 0) + { + // Remeber the last save time, since this is the predicate. + const auto lastSaveTime = _lastSaveTime; - return sent; + if (_saveCV.wait_for(lock, std::chrono::milliseconds(waitTimeoutMs)) == std::cv_status::no_timeout) + { + return true; } + + return (lastSaveTime != _lastSaveTime); } - return false; + return sent; } -bool DocumentBroker::waitSave(const size_t timeoutMs) +bool DocumentBroker::sendUnoSave() { - std::unique_lock<std::mutex> lock(_saveMutex); - - // Remeber the last save time, since this is the predicate. - const auto lastSaveTime = _lastSaveTime; + Log::info("Autosave triggered for doc [" + _docKey + "]."); + assert(!_mutex.try_lock()); - if (_saveCV.wait_for(lock, std::chrono::milliseconds(timeoutMs)) == std::cv_status::no_timeout) + // Save using session holding the edit-lock + for (auto& sessionIt: _sessions) { - return true; + if (sessionIt.second->isEditLocked()) + { + auto queue = sessionIt.second->getQueue(); + if (queue) + { + queue->put("uno .uno:Save"); + return true; + } + } } - return (lastSaveTime != _lastSaveTime); + Log::error("Failed to auto-save doc [" + _docKey + "]: No valid sessions."); + return false; } std::string DocumentBroker::getJailRoot() const diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp index 1ef6df0..d8eff26 100644 --- a/loolwsd/DocumentBroker.hpp +++ b/loolwsd/DocumentBroker.hpp @@ -152,19 +152,19 @@ public: /// Loads a document from the public URI into the jail. bool load(const std::string& jailId); + /// Save the document to Storage if needs persisting. bool save(); bool isModified() const { return _isModified; } void setModified(const bool value); - /// Save the document if there was activity since last save. - /// force when true, will force saving immediatly, regardless - /// of how long ago the activity was. - bool autoSave(const bool force); - - /// Wait until the document is saved next. - /// This is used to cleanup after the last save. - /// Returns false if times out. - bool waitSave(const size_t timeoutMs); + /// Save the document if the document is modified. + /// force when true, will force saving if there + /// has been any recent activity after the last save. + /// waitTimeoutMs when >0 will wait for the save to + /// complete before returning, or timeout. + /// Returns true if attempts to save or it also waits + /// and receives save notification. Otherwise, false. + bool autoSave(const bool force, const size_t waitTimeoutMs); Poco::URI getPublicUri() const { return _uriPublic; } Poco::URI getJailedUri() const { return _uriJailed; } @@ -204,6 +204,14 @@ public: bool isMarkedToDestroy() const { return _markToDestroy; } private: + + /// Sends the .uno:Save command to LoKit. + bool sendUnoSave(); + + /// Saves the document to Storage (assuming LO Core saved to local copy). + bool saveToStorage(); + +private: const Poco::URI _uriPublic; const std::string _docKey; const std::string _childRoot; diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp index 3acc19b..be728a6 100644 --- a/loolwsd/LOOLWSD.cpp +++ b/loolwsd/LOOLWSD.cpp @@ -604,10 +604,7 @@ private: // Use auto-save to save only when there are modifications since last save. // We also need to wait until the save notification reaches us // and Storage persists the document. - // Note: technically, there is a race between these two (we should - // hold the broker lock before issueing the save and waiting,) - // but in practice this shouldn't happen. - if (docBroker->autoSave(true) && !docBroker->waitSave(COMMAND_TIMEOUT_MS)) + if (docBroker->autoSave(true, COMMAND_TIMEOUT_MS)) { Log::error("Auto-save before closing failed."); } @@ -1581,7 +1578,7 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/) { if (brokerIt.second->isModified()) { - brokerIt.second->autoSave(false); + brokerIt.second->autoSave(false, 0); } } } _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
