loolwsd/DocumentBroker.cpp | 16 ++++++++--------
loolwsd/LOOLWSD.cpp | 12 ++++++++----
2 files changed, 16 insertions(+), 12 deletions(-)
New commits:
commit 702dd48f1f7f671d918fbadcf98fb4ae4743dbbe
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date: Sun May 8 10:07:17 2016 -0400
loolwsd: safer document saving
Relying on the filesystem to tell us when the document
was last modified (to decide whether to upload to storage or not,)
proved unreliable.
Now we always upload to storage if there is only one client.
This both minimizes the risk and also avoids the file timestamp
check as a workaround to the problem of re-uploading documents
as many time as there were clients. Since with one client we
can only upload no more than once per save, which is reasonable.
Furthermore, when a client disconnects we auto-save automatically
as a matter of precaution. However, when there are other clients
still connected, we don't wait for the save to complete, rather
we let that job to the very last one.
Change-Id: I94a2e4bddaed30a6c9c0e69f8006667d33c5b8ee
Reviewed-on: https://gerrit.libreoffice.org/24767
Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
Tested-by: Ashod Nakashian <ashnak...@gmail.com>
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 4c8591a..adde590 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -177,7 +177,9 @@ bool DocumentBroker::save()
const auto uri = _uriPublic.toString();
- // If the file hasn't been modified within the past 10 seconds, skip
saving.
+ // If we aren't potentially destroying just yet, and the file hasn't been
+ // modified within the past 10 seconds, skip saving.
+ //
// FIXME this is because currently the ChildProcessSession broadcasts the
// unocommandresult, so we get called several times here, and have no real
// possibility to distinguish who was the 1st caller.
@@ -187,10 +189,11 @@ bool DocumentBroker::save()
// is planned post-release.
const auto newFileModifiedTime =
Poco::File(_storage->getLocalRootPath()).getLastModified();
const auto elapsed = newFileModifiedTime - _lastFileModifiedTime;
- if (std::abs(elapsed) > 10 * 1000 * 1000)
+ if (!canDestroy() && std::abs(elapsed) > 10 * 1000 * 1000)
{
// Nothing to do.
- Log::debug("Skipping unnecessary saving to URI [" + uri + "].");
+ Log::debug() << "Skipping unnecessary saving to URI [" << uri
+ << "]. File last modified " << elapsed << " ms ago." <<
Log::end;
return true;
}
@@ -502,11 +505,8 @@ bool DocumentBroker::canDestroy()
{
std::unique_lock<std::mutex> lock(_mutex);
- if (_sessions.size() == 1)
- {
- // Last view going away, can destroy.
- _markToDestroy = true;
- }
+ // Last view going away, can destroy.
+ _markToDestroy = (_sessions.size() <= 1);
return _markToDestroy;
}
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 37c71a9..25a3801 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -648,15 +648,19 @@ private:
[&session]() { session->closeFrame(); },
[&queueHandlerThread]() { return TerminationFlag ||
!queueHandlerThread.isRunning(); });
- const bool canDestroy = docBroker->canDestroy();
- if (canDestroy && !session->_bLoadError)
+ if (!session->_bLoadError)
{
- Log::info("Shutdown of the last session, saving the document
before tearing down.");
+ // If we are the last, we must wait for the save to complete.
+ const bool canDestroy = docBroker->canDestroy();
+ if (canDestroy)
+ {
+ Log::info("Shutdown of the last session, saving the
document before tearing down.");
+ }
// 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.
- if (!docBroker->autoSave(true, COMMAND_TIMEOUT_MS))
+ if (!docBroker->autoSave(canDestroy, COMMAND_TIMEOUT_MS))
{
Log::error("Auto-save before closing failed.");
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
