Fix a crash when saving the locked Word document file due to unnecessary spam to the lock state change from folder watcher logic.

Signed-off-by: alex-z <blackslayer4@gmail.com>
This commit is contained in:
alex-z 2023-09-04 20:55:02 +02:00
parent 270f6bf712
commit 3815919aa5
10 changed files with 184 additions and 51 deletions

View file

@ -647,6 +647,8 @@ void EditLocallyJob::lockFile()
this, &EditLocallyJob::fileLockError));
_folderForFile->accountState()->account()->setLockFileState(_relPath,
_folderForFile->remotePathTrailingSlash(),
_folderForFile->path(),
_folderForFile->journalDb(),
SyncFileItem::LockStatus::LockedItem);
}

View file

@ -666,7 +666,11 @@ void Folder::slotFilesLockReleased(const QSet<QString> &files)
disconnect(_officeFileLockReleaseUnlockFailure);
qCWarning(lcFolder) << "Failed to unlock a file:" << remoteFilePath << message;
});
_accountState->account()->setLockFileState(remoteFilePath, journalDb(), SyncFileItem::LockStatus::UnlockedItem);
_accountState->account()->setLockFileState(remoteFilePath,
remotePathTrailingSlash(),
path(),
journalDb(),
SyncFileItem::LockStatus::UnlockedItem);
}
}
@ -690,10 +694,7 @@ void Folder::slotLockedFilesFound(const QSet<QString> &files)
const auto fileRecordPath = fileFromLocalPath(file);
SyncJournalFileRecord rec;
const auto canLockFile = journalDb()->getFileRecord(fileRecordPath, &rec) && rec.isValid()
&& (!rec._lockstate._locked
|| (rec._lockstate._lockOwnerType == static_cast<qint64>(SyncFileItem::LockOwnerType::UserLock)
&& rec._lockstate._lockOwnerId == _accountState->account()->davUser()));
const auto canLockFile = journalDb()->getFileRecord(fileRecordPath, &rec) && rec.isValid() && !rec._lockstate._locked;
if (!canLockFile) {
qCDebug(lcFolder) << "Skipping locking file" << file << "with rec.isValid():" << rec.isValid()
@ -712,7 +713,11 @@ void Folder::slotLockedFilesFound(const QSet<QString> &files)
disconnect(_fileLockFailure);
qCWarning(lcFolder) << "Failed to lock a file:" << remoteFilePath << message;
});
_accountState->account()->setLockFileState(remoteFilePath, journalDb(), SyncFileItem::LockStatus::LockedItem);
_accountState->account()->setLockFileState(remoteFilePath,
remotePathTrailingSlash(),
path(),
journalDb(),
SyncFileItem::LockStatus::LockedItem);
}
}

View file

@ -35,7 +35,6 @@
#include <QDir>
#include <QMutexLocker>
#include <QStringList>
#include <QTimer>
#include <array>
#include <cstdint>
@ -44,6 +43,8 @@ namespace
{
const std::array<const char *, 2> lockFilePatterns = {{".~lock.", "~$"}};
constexpr auto lockChangeDebouncingTimerIntervalMs = 500;
QString filePathLockFilePatternMatch(const QString &path)
{
qCDebug(OCC::lcFolderWatcher) << "Checking if it is a lock file:" << path;
@ -78,6 +79,8 @@ FolderWatcher::FolderWatcher(Folder *folder)
: QObject(folder)
, _folder(folder)
{
_lockChangeDebouncingTimer.setInterval(lockChangeDebouncingTimerIntervalMs);
if (_folder && _folder->accountState() && _folder->accountState()->account()) {
connect(_folder->accountState()->account().data(), &Account::capabilitiesChanged, this, &FolderWatcher::folderAccountCapabilitiesChanged);
folderAccountCapabilitiesChanged();
@ -157,6 +160,20 @@ void FolderWatcher::startNotificationTestWhenReady()
});
}
void FolderWatcher::lockChangeDebouncingTimerTimedOut()
{
if (!_unlockedFiles.isEmpty()) {
const auto unlockedFilesCopy = _unlockedFiles;
emit filesLockReleased(unlockedFilesCopy);
_unlockedFiles.clear();
}
if (!_lockedFiles.isEmpty()) {
const auto lockedFilesCopy = _lockedFiles;
emit filesLockImposed(lockedFilesCopy);
emit lockedFilesFound(lockedFilesCopy);
_lockedFiles.clear();
}
}
int FolderWatcher::testLinuxWatchCount() const
{
@ -195,8 +212,6 @@ void FolderWatcher::changeDetected(const QStringList &paths)
_timer.restart();
QSet<QString> changedPaths;
QSet<QString> unlockedFiles;
QSet<QString> lockedFiles;
for (const auto &path : paths) {
if (!_testNotificationPath.isEmpty()
@ -208,18 +223,18 @@ void FolderWatcher::changeDetected(const QStringList &paths)
const auto checkResult = lockFileTargetFilePath(path,lockFileNamePattern);
if (_shouldWatchForFileUnlocking) {
// Lock file has been deleted, file now unlocked
if (checkResult.type == FileLockingInfo::Type::Unlocked && !checkResult.path.isEmpty() && !QFile::exists(path)) {
unlockedFiles.insert(checkResult.path);
} else if (!checkResult.path.isEmpty() && QFile::exists(path)) { // Lock file found
lockedFiles.insert(checkResult.path);
if (checkResult.type == FileLockingInfo::Type::Unlocked && !checkResult.path.isEmpty()) {
_lockedFiles.remove(checkResult.path);
_unlockedFiles.insert(checkResult.path);
}
}
if (checkResult.type == FileLockingInfo::Type::Locked && !checkResult.path.isEmpty()) {
lockedFiles.insert(checkResult.path);
_unlockedFiles.remove(checkResult.path);
_lockedFiles.insert(checkResult.path);
}
qCDebug(lcFolderWatcher) << "Locked files:" << lockedFiles.values();
qCDebug(lcFolderWatcher) << "Locked files:" << _lockedFiles.values();
// ------- handle ignores:
if (pathIsIgnored(path)) {
@ -229,19 +244,16 @@ void FolderWatcher::changeDetected(const QStringList &paths)
changedPaths.insert(path);
}
qCDebug(lcFolderWatcher) << "Unlocked files:" << unlockedFiles.values();
qCDebug(lcFolderWatcher) << "Locked files:" << lockedFiles;
qCDebug(lcFolderWatcher) << "Unlocked files:" << _unlockedFiles.values();
qCDebug(lcFolderWatcher) << "Locked files:" << _lockedFiles;
if (!unlockedFiles.isEmpty()) {
emit filesLockReleased(unlockedFiles);
}
if (!lockedFiles.isEmpty()) {
emit filesLockImposed(lockedFiles);
}
if (!lockedFiles.isEmpty()) {
emit lockedFilesFound(lockedFiles);
if (!_lockedFiles.isEmpty() || !_unlockedFiles.isEmpty()) {
if (_lockChangeDebouncingTimer.isActive()) {
_lockChangeDebouncingTimer.stop();
}
_lockChangeDebouncingTimer.setSingleShot(true);
_lockChangeDebouncingTimer.start();
_lockChangeDebouncingTimer.connect(&_lockChangeDebouncingTimer, &QTimer::timeout, this, &FolderWatcher::lockChangeDebouncingTimerTimedOut, Qt::UniqueConnection);
}
if (changedPaths.isEmpty()) {

View file

@ -27,8 +27,7 @@
#include <QScopedPointer>
#include <QSet>
#include <QDir>
class QTimer;
#include <QTimer>
namespace OCC {
@ -130,6 +129,7 @@ protected slots:
private slots:
void startNotificationTestWhenReady();
void lockChangeDebouncingTimerTimedOut();
protected:
QHash<QString, int> _pendingPathes;
@ -156,6 +156,11 @@ private:
/** Path of the expected test notification */
QString _testNotificationPath;
QSet<QString> _unlockedFiles;
QSet<QString> _lockedFiles;
QTimer _lockChangeDebouncingTimer;
friend class FolderWatcherPrivate;
};
}

View file

@ -1091,7 +1091,11 @@ void SocketApi::setFileLock(const QString &localFile, const SyncFileItem::LockSt
return;
}
shareFolder->accountState()->account()->setLockFileState(fileData.serverRelativePath, shareFolder->journalDb(), lockState);
shareFolder->accountState()->account()->setLockFileState(fileData.serverRelativePath,
shareFolder->remotePathTrailingSlash(),
shareFolder->path(),
shareFolder->journalDb(),
lockState);
shareFolder->journalDb()->schedulePathForRemoteDiscovery(fileData.serverRelativePath);
shareFolder->scheduleThisFolderSoon();

View file

@ -913,6 +913,17 @@ void Account::slotDirectEditingRecieved(const QJsonDocument &json)
}
}
void Account::removeLockStatusChangeInprogress(const QString &serverRelativePath, const SyncFileItem::LockStatus lockStatus)
{
const auto foundLockStatusJobInProgress = _lockStatusChangeInprogress.find(serverRelativePath);
if (foundLockStatusJobInProgress != _lockStatusChangeInprogress.end()) {
foundLockStatusJobInProgress.value().removeAll(lockStatus);
if (foundLockStatusJobInProgress.value().isEmpty()) {
_lockStatusChangeInprogress.erase(foundLockStatusJobInProgress);
}
}
}
PushNotifications *Account::pushNotifications() const
{
return _pushNotifications;
@ -924,14 +935,24 @@ std::shared_ptr<UserStatusConnector> Account::userStatusConnector() const
}
void Account::setLockFileState(const QString &serverRelativePath,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
SyncJournalDb * const journal,
const SyncFileItem::LockStatus lockStatus)
{
auto job = std::make_unique<LockFileJob>(sharedFromThis(), journal, serverRelativePath, lockStatus);
connect(job.get(), &LockFileJob::finishedWithoutError, this, [this]() {
auto& lockStatusJobInProgress = _lockStatusChangeInprogress[serverRelativePath];
if (lockStatusJobInProgress.contains(lockStatus)) {
qCWarning(lcAccount) << "Already running a job with lockStatus:" << lockStatus << " for: " << serverRelativePath;
return;
}
lockStatusJobInProgress.push_back(lockStatus);
auto job = std::make_unique<LockFileJob>(sharedFromThis(), journal, serverRelativePath, remoteSyncPathWithTrailingSlash, localSyncPath, lockStatus);
connect(job.get(), &LockFileJob::finishedWithoutError, this, [this, serverRelativePath, lockStatus]() {
removeLockStatusChangeInprogress(serverRelativePath, lockStatus);
Q_EMIT lockFileSuccess();
});
connect(job.get(), &LockFileJob::finishedWithError, this, [lockStatus, serverRelativePath, this](const int httpErrorCode, const QString &errorString, const QString &lockOwnerName) {
removeLockStatusChangeInprogress(serverRelativePath, lockStatus);
auto errorMessage = QString{};
const auto filePath = serverRelativePath.mid(1);

View file

@ -308,6 +308,8 @@ public:
[[nodiscard]] std::shared_ptr<UserStatusConnector> userStatusConnector() const;
void setLockFileState(const QString &serverRelativePath,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
SyncJournalDb * const journal,
const SyncFileItem::LockStatus lockStatus);
@ -373,6 +375,9 @@ protected Q_SLOTS:
void slotCredentialsAsked();
void slotDirectEditingRecieved(const QJsonDocument &json);
private slots:
void removeLockStatusChangeInprogress(const QString &serverRelativePath, const SyncFileItem::LockStatus lockStatus);
private:
Account(QObject *parent = nullptr);
void setSharedThis(AccountPtr sharedThis);
@ -436,6 +441,8 @@ private:
std::shared_ptr<UserStatusConnector> _userStatusConnector;
QHash<QString, QVector<SyncFileItem::LockStatus>> _lockStatusChangeInprogress;
/* IMPORTANT - remove later - FIXME MS@2019-12-07 -->
* TODO: For "Log out" & "Remove account": Remove client CA certs and KEY!
*

View file

@ -28,12 +28,19 @@ Q_LOGGING_CATEGORY(lcLockFileJob, "nextcloud.sync.networkjob.lockfile", QtInfoMs
LockFileJob::LockFileJob(const AccountPtr account,
SyncJournalDb* const journal,
const QString &path,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
const SyncFileItem::LockStatus requestedLockState,
QObject *parent)
: AbstractNetworkJob(account, path, parent)
, _journal(journal)
, _requestedLockState(requestedLockState)
, _remoteSyncPathWithTrailingSlash(remoteSyncPathWithTrailingSlash)
, _localSyncPath(localSyncPath)
{
if (!_localSyncPath.endsWith(QLatin1Char('/'))) {
_localSyncPath.append(QLatin1Char('/'));
}
}
void LockFileJob::start()
@ -164,12 +171,12 @@ SyncJournalFileRecord LockFileJob::handleReply()
}
}
const auto relativePath = path().mid(1);
if (_journal->getFileRecord(relativePath, &record) && record.isValid()) {
const auto relativePathInDb = path().mid(_remoteSyncPathWithTrailingSlash.size());
if (_journal->getFileRecord(relativePathInDb, &record) && record.isValid()) {
setFileRecordLocked(record);
if (_lockOwnerType != SyncFileItem::LockOwnerType::UserLock ||
_userId != account()->davUser()) {
FileSystem::setFileReadOnly(relativePath, true);
if ((_lockStatus == SyncFileItem::LockStatus::LockedItem)
&& (_lockOwnerType != SyncFileItem::LockOwnerType::UserLock || _userId != account()->davUser())) {
FileSystem::setFileReadOnly(_localSyncPath + relativePathInDb, true);
}
const auto result = _journal->setFileRecord(record);
if (!result) {

View file

@ -22,6 +22,8 @@ public:
explicit LockFileJob(const AccountPtr account,
SyncJournalDb* const journal,
const QString &path,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
const SyncFileItem::LockStatus requestedLockState,
QObject *parent = nullptr);
void start() override;
@ -54,6 +56,8 @@ private:
QString _userId;
qint64 _lockTime = 0;
qint64 _lockTimeout = 0;
QString _remoteSyncPathWithTrailingSlash;
QString _localSyncPath;
};
}

View file

@ -36,7 +36,11 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName, &fakeFolder.syncJournal(), OCC::SyncFileItem::LockStatus::LockedItem);
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
QVERIFY(lockFileSuccessSpy.wait());
QCOMPARE(lockFileErrorSpy.count(), 0);
@ -77,7 +81,11 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName, &fakeFolder.syncJournal(), OCC::SyncFileItem::LockStatus::LockedItem);
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
QVERIFY(lockFileErrorSpy.wait());
QCOMPARE(lockFileSuccessSpy.count(), 0);
@ -97,7 +105,11 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName, &fakeFolder.syncJournal(), OCC::SyncFileItem::LockStatus::LockedItem);
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
QVERIFY(lockFileSuccessSpy.wait());
QCOMPARE(lockFileErrorSpy.count(), 0);
@ -120,7 +132,11 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName, &fakeFolder.syncJournal(), OCC::SyncFileItem::LockStatus::LockedItem);
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
QVERIFY(lockFileSuccessSpy.wait());
QCOMPARE(lockFileErrorSpy.count(), 0);
@ -139,7 +155,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto job = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::LockedItem);
auto job = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
@ -172,7 +193,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto lockFileJob = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::LockedItem);
auto lockFileJob = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
QSignalSpy lockFileJobSuccess(lockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy lockFileJobFailure(lockFileJob, &OCC::LockFileJob::finishedWithError);
@ -184,7 +210,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto unlockFileJob = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::UnlockedItem);
auto unlockFileJob = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
QSignalSpy unlockFileJobSuccess(unlockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy unlockFileJobFailure(unlockFileJob, &OCC::LockFileJob::finishedWithError);
@ -238,7 +269,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto job = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::LockedItem);
auto job = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
@ -286,7 +322,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto job = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::LockedItem);
auto job = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
@ -334,7 +375,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto job = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::UnlockedItem);
auto job = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
@ -380,7 +426,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto job = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::UnlockedItem);
auto job = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
@ -413,7 +464,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto lockFileJob = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::LockedItem);
auto lockFileJob = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
QSignalSpy lockFileJobSuccess(lockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy lockFileJobFailure(lockFileJob, &OCC::LockFileJob::finishedWithError);
@ -425,7 +481,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto unlockFileJob = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::UnlockedItem);
auto unlockFileJob = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
QSignalSpy unlockFileJobSuccess(unlockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy unlockFileJobFailure(unlockFileJob, &OCC::LockFileJob::finishedWithError);
@ -473,7 +534,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
auto job = new OCC::LockFileJob(fakeFolder.account(), &fakeFolder.syncJournal(), QStringLiteral("/") + testFileName, OCC::SyncFileItem::LockStatus::UnlockedItem);
auto job = new OCC::LockFileJob(fakeFolder.account(),
&fakeFolder.syncJournal(),
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);