[Libreoffice-commits] online.git: loolwsd/ChildProcessSession.cpp loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Tue Apr 26 10:40:53 UTC 2016
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 <ashod.nakashian at collabora.co.uk>
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 <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
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);
}
}
}
More information about the Libreoffice-commits
mailing list