Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2024-12-27 00:09:53 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fix potential deadlock in UI when saving

Browse Source 
This was noted as a problem in several issues and it finally occurred to me and I traced it to the fact that a timing issue sometimes allowed the file watcher to trigger a "file changed" alert right when saving starts. I fixed this by moving where the mutex lock is made for saving and preventing database reload during a save operation.
This commit is contained in:
Jonathan White 2022-10-18 18:23:46 -04:00
parent e6b2e4e95e
commit e180980b90
2 changed files with 6 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/core/Database.cpp
View File

@ -253,9 +253,6 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString&
return false; return false;
} }
// Prevent destructive operations while saving
QMutexLocker locker(&m_saveMutex);
if (filePath == m_data.filePath) { if (filePath == m_data.filePath) {
// Fail-safe check to make sure we don't overwrite underlying file changes // Fail-safe check to make sure we don't overwrite underlying file changes
// that have not yet triggered a file reload/merge operation. // that have not yet triggered a file reload/merge operation.
@ -270,6 +267,9 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString&
// Clear read-only flag // Clear read-only flag
m_fileWatcher->stop(); m_fileWatcher->stop();
// Prevent destructive operations while saving
QMutexLocker locker(&m_saveMutex);
QFileInfo fileInfo(filePath); QFileInfo fileInfo(filePath);
auto realFilePath = fileInfo.exists() ? fileInfo.canonicalFilePath() : fileInfo.absoluteFilePath(); auto realFilePath = fileInfo.exists() ? fileInfo.canonicalFilePath() : fileInfo.absoluteFilePath();
bool isNewFile = !QFile::exists(realFilePath); bool isNewFile = !QFile::exists(realFilePath);
@ -463,6 +463,7 @@ bool Database::import(const QString& xmlExportPath, QString* error)
void Database::releaseData() void Database::releaseData()
{ {
// Prevent data release while saving // Prevent data release while saving
Q_ASSERT(!isSaving());
QMutexLocker locker(&m_saveMutex); QMutexLocker locker(&m_saveMutex);
if (m_modified) { if (m_modified) {

4
src/gui/DatabaseWidget.cpp
View File

@ -1747,8 +1747,8 @@ bool DatabaseWidget::lock()
void DatabaseWidget::reloadDatabaseFile() void DatabaseWidget::reloadDatabaseFile()
{ {
// Ignore reload if we are locked or currently editing an entry or group // Ignore reload if we are locked, saving, or currently editing an entry or group
if (!m_db || isLocked() || isEntryEditActive() || isGroupEditActive()) { if (!m_db || isLocked() || isEntryEditActive() || isGroupEditActive() || isSaving()) {
return; return;
} }