From 711359ef43e56145bdf76130a439f82002df1456 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Thu, 13 Apr 2023 18:49:48 +0800 Subject: [PATCH 1/4] Ensure we are closing current log file before entering next log file Signed-off-by: Claudio Cambra --- src/libsync/logger.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index fe8acf25e7..54347f3811 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -119,6 +119,7 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString #endif { if (_logFile.size() >= MaxLogSizeBytes) { + close(); enterNextLogFile(); } From 1f1d9d6dbae58a928337e126e9d63abeb6927300 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Thu, 13 Apr 2023 18:50:50 +0800 Subject: [PATCH 2/4] Ensure we are making all file modifications under mutex protection Signed-off-by: Claudio Cambra --- src/libsync/logger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index 54347f3811..42830d32f3 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -118,12 +118,12 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString cout << msg << endl; #endif { + QMutexLocker lock(&_mutex); if (_logFile.size() >= MaxLogSizeBytes) { close(); enterNextLogFile(); } - QMutexLocker lock(&_mutex); _crashLogIndex = (_crashLogIndex + 1) % CrashLogSize; _crashLog[_crashLogIndex] = msg; From c2d72109b626c656b095134da532e9250ca2dda7 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 13 Apr 2023 18:41:44 +0200 Subject: [PATCH 3/4] protext all access to _logstream with the existing mutex Signed-off-by: Matthieu Gallien --- src/libsync/logger.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index 42830d32f3..d38be4c6bf 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -118,7 +118,6 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString cout << msg << endl; #endif { - QMutexLocker lock(&_mutex); if (_logFile.size() >= MaxLogSizeBytes) { close(); enterNextLogFile(); @@ -127,6 +126,8 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString _crashLogIndex = (_crashLogIndex + 1) % CrashLogSize; _crashLog[_crashLogIndex] = msg; + QMutexLocker lock(&_mutex); + if (_logstream) { (*_logstream) << msg << Qt::endl; if (_doFileFlush) @@ -145,6 +146,7 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString void Logger::close() { + QMutexLocker lock(&_mutex); dumpCrashLog(); if (_logstream) { From 9c8b624ef7350f649e06e0f16249b0e4e800b4f1 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 18 Apr 2023 23:30:34 +0200 Subject: [PATCH 4/4] protect all access to shared data by mutexes separate public methods that must lock and private methods that must not lock will avoid deadlock or unprotected accesses Signed-off-by: Matthieu Gallien --- src/libsync/logger.cpp | 139 ++++++++++++++++++++++------------------- src/libsync/logger.h | 4 +- 2 files changed, 79 insertions(+), 64 deletions(-) diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp index d38be4c6bf..2bed41214c 100644 --- a/src/libsync/logger.cpp +++ b/src/libsync/logger.cpp @@ -34,9 +34,38 @@ #endif namespace { + constexpr int CrashLogSize = 20; constexpr int MaxLogSizeBytes = 1024 * 512; + +static bool compressLog(const QString &originalName, const QString &targetName) +{ +#ifdef ZLIB_FOUND + QFile original(originalName); + if (!original.open(QIODevice::ReadOnly)) + return false; + auto compressed = gzopen(targetName.toUtf8(), "wb"); + if (!compressed) { + return false; + } + + while (!original.atEnd()) { + auto data = original.read(1024 * 1024); + auto written = gzwrite(compressed, data.data(), data.size()); + if (written != data.size()) { + gzclose(compressed); + return false; + } + } + gzclose(compressed); + return true; +#else + return false; +#endif } + +} + namespace OCC { Logger *Logger::instance() @@ -118,23 +147,23 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString cout << msg << endl; #endif { + QMutexLocker lock(&_mutex); + if (_logFile.size() >= MaxLogSizeBytes) { - close(); - enterNextLogFile(); + closeNoLock(); + enterNextLogFileNoLock(); } _crashLogIndex = (_crashLogIndex + 1) % CrashLogSize; _crashLog[_crashLogIndex] = msg; - QMutexLocker lock(&_mutex); - if (_logstream) { (*_logstream) << msg << Qt::endl; if (_doFileFlush) _logstream->flush(); } if (type == QtFatalMsg) { - close(); + closeNoLock(); #if defined(Q_OS_WIN) // Make application terminate in a way that can be caught by the crash reporter Utility::crash(); @@ -144,9 +173,8 @@ void Logger::doLog(QtMsgType type, const QMessageLogContext &ctx, const QString emit logWindowLog(msg); } -void Logger::close() +void Logger::closeNoLock() { - QMutexLocker lock(&_mutex); dumpCrashLog(); if (_logstream) { @@ -158,40 +186,14 @@ void Logger::close() QString Logger::logFile() const { + QMutexLocker locker(&_mutex); return _logFile.fileName(); } void Logger::setLogFile(const QString &name) { QMutexLocker locker(&_mutex); - if (_logstream) { - _logstream.reset(nullptr); - _logFile.close(); - } - - if (name.isEmpty()) { - return; - } - - bool openSucceeded = false; - if (name == QLatin1String("-")) { - openSucceeded = _logFile.open(stdout, QIODevice::WriteOnly); - } else { - _logFile.setFileName(name); - openSucceeded = _logFile.open(QIODevice::WriteOnly); - } - - if (!openSucceeded) { - locker.unlock(); // Just in case postGuiMessage has a qDebug() - postGuiMessage(tr("Error"), - QString(tr("File \"%1\"
cannot be opened for writing.

" - "The log output cannot be saved!")) - .arg(name)); - return; - } - - _logstream.reset(new QTextStream(&_logFile)); - _logstream->setCodec(QTextCodec::codecForName("UTF-8")); + setLogFileNoLock(name); } void Logger::setLogExpire(int expire) @@ -276,33 +278,7 @@ void Logger::dumpCrashLog() } } -static bool compressLog(const QString &originalName, const QString &targetName) -{ -#ifdef ZLIB_FOUND - QFile original(originalName); - if (!original.open(QIODevice::ReadOnly)) - return false; - auto compressed = gzopen(targetName.toUtf8(), "wb"); - if (!compressed) { - return false; - } - - while (!original.atEnd()) { - auto data = original.read(1024 * 1024); - auto written = gzwrite(compressed, data.data(), data.size()); - if (written != data.size()) { - gzclose(compressed); - return false; - } - } - gzclose(compressed); - return true; -#else - return false; -#endif -} - -void Logger::enterNextLogFile() +void Logger::enterNextLogFileNoLock() { if (!_logDirectory.isEmpty()) { @@ -335,7 +311,7 @@ void Logger::enterNextLogFile() newLogName.append("." + QString::number(maxNumber + 1)); auto previousLog = _logFile.fileName(); - setLogFile(dir.filePath(newLogName)); + setLogFileNoLock(dir.filePath(newLogName)); // Compress the previous log file. On a restart this can be the most recent // log file. @@ -353,4 +329,41 @@ void Logger::enterNextLogFile() } } +void Logger::setLogFileNoLock(const QString &name) +{ + if (_logstream) { + _logstream.reset(nullptr); + _logFile.close(); + } + + if (name.isEmpty()) { + return; + } + + bool openSucceeded = false; + if (name == QLatin1String("-")) { + openSucceeded = _logFile.open(stdout, QIODevice::WriteOnly); + } else { + _logFile.setFileName(name); + openSucceeded = _logFile.open(QIODevice::WriteOnly); + } + + if (!openSucceeded) { + postGuiMessage(tr("Error"), + QString(tr("File \"%1\"
cannot be opened for writing.

" + "The log output cannot be saved!")) + .arg(name)); + return; + } + + _logstream.reset(new QTextStream(&_logFile)); + _logstream->setCodec(QTextCodec::codecForName("UTF-8")); +} + +void Logger::enterNextLogFile() +{ + QMutexLocker locker(&_mutex); + enterNextLogFileNoLock(); +} + } // namespace OCC diff --git a/src/libsync/logger.h b/src/libsync/logger.h index c37da97faa..5fd14431e5 100644 --- a/src/libsync/logger.h +++ b/src/libsync/logger.h @@ -96,8 +96,10 @@ private: Logger(QObject *parent = nullptr); ~Logger() override; - void close(); + void closeNoLock(); void dumpCrashLog(); + void enterNextLogFileNoLock(); + void setLogFileNoLock(const QString &name); QFile _logFile; bool _doFileFlush = false;