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 <ashnakash@gmail.com> Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This commit is contained in:
parent
ce36141662
commit
702dd48f1f
2 changed files with 16 additions and 12 deletions
|
@ -177,7 +177,9 @@ bool DocumentBroker::save()
|
||||||
|
|
||||||
const auto uri = _uriPublic.toString();
|
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
|
// FIXME this is because currently the ChildProcessSession broadcasts the
|
||||||
// unocommandresult, so we get called several times here, and have no real
|
// unocommandresult, so we get called several times here, and have no real
|
||||||
// possibility to distinguish who was the 1st caller.
|
// possibility to distinguish who was the 1st caller.
|
||||||
|
@ -187,10 +189,11 @@ bool DocumentBroker::save()
|
||||||
// is planned post-release.
|
// is planned post-release.
|
||||||
const auto newFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified();
|
const auto newFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified();
|
||||||
const auto elapsed = newFileModifiedTime - _lastFileModifiedTime;
|
const auto elapsed = newFileModifiedTime - _lastFileModifiedTime;
|
||||||
if (std::abs(elapsed) > 10 * 1000 * 1000)
|
if (!canDestroy() && std::abs(elapsed) > 10 * 1000 * 1000)
|
||||||
{
|
{
|
||||||
// Nothing to do.
|
// 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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -502,11 +505,8 @@ bool DocumentBroker::canDestroy()
|
||||||
{
|
{
|
||||||
std::unique_lock<std::mutex> lock(_mutex);
|
std::unique_lock<std::mutex> lock(_mutex);
|
||||||
|
|
||||||
if (_sessions.size() == 1)
|
// Last view going away, can destroy.
|
||||||
{
|
_markToDestroy = (_sessions.size() <= 1);
|
||||||
// Last view going away, can destroy.
|
|
||||||
_markToDestroy = true;
|
|
||||||
}
|
|
||||||
|
|
||||||
return _markToDestroy;
|
return _markToDestroy;
|
||||||
}
|
}
|
||||||
|
|
|
@ -648,15 +648,19 @@ private:
|
||||||
[&session]() { session->closeFrame(); },
|
[&session]() { session->closeFrame(); },
|
||||||
[&queueHandlerThread]() { return TerminationFlag || !queueHandlerThread.isRunning(); });
|
[&queueHandlerThread]() { return TerminationFlag || !queueHandlerThread.isRunning(); });
|
||||||
|
|
||||||
const bool canDestroy = docBroker->canDestroy();
|
if (!session->_bLoadError)
|
||||||
if (canDestroy && !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.
|
// Use auto-save to save only when there are modifications since last save.
|
||||||
// We also need to wait until the save notification reaches us
|
// We also need to wait until the save notification reaches us
|
||||||
// and Storage persists the document.
|
// 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.");
|
Log::error("Auto-save before closing failed.");
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue