From 22ebd42c328fbf970daf3073b531fdc0fb6be787 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Thu, 6 Feb 2025 11:22:52 -0500 Subject: [PATCH] Misc fixes for undefined behavior, crashes, and build failure (#3465) Signed-off-by: Jared Van Bortel --- gpt4all-backend/src/llmodel.cpp | 9 ++++- gpt4all-chat/CHANGELOG.md | 6 +++ gpt4all-chat/src/chatmodel.h | 3 +- gpt4all-chat/src/database.cpp | 45 +++++++++++----------- gpt4all-chat/src/database.h | 2 +- gpt4all-chat/src/embllm.cpp | 7 +++- gpt4all-chat/src/logger.cpp | 6 ++- gpt4all-chat/src/logger.h | 18 +++++---- gpt4all-chat/src/network.cpp | 21 ++++++++++ gpt4all-chat/translations/gpt4all_ro_RO.ts | 5 --- 10 files changed, 81 insertions(+), 41 deletions(-) diff --git a/gpt4all-backend/src/llmodel.cpp b/gpt4all-backend/src/llmodel.cpp index ee247f35..de130593 100644 --- a/gpt4all-backend/src/llmodel.cpp +++ b/gpt4all-backend/src/llmodel.cpp @@ -140,9 +140,14 @@ const std::vector &LLModel::Implementation::implementat std::string path; // Split the paths string by the delimiter and process each path. while (std::getline(ss, path, ';')) { - std::u8string u8_path(path.begin(), path.end()); + fs::directory_iterator iter; + try { + iter = fs::directory_iterator(std::u8string(path.begin(), path.end())); + } catch (const fs::filesystem_error &) { + continue; // skip nonexistent path + } // Iterate over all libraries - for (const auto &f : fs::directory_iterator(u8_path)) { + for (const auto &f : iter) { const fs::path &p = f.path(); if (p.extension() != LIB_FILE_EXT) continue; diff --git a/gpt4all-chat/CHANGELOG.md b/gpt4all-chat/CHANGELOG.md index 7ed12dc1..7de937b9 100644 --- a/gpt4all-chat/CHANGELOG.md +++ b/gpt4all-chat/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [Unreleased] + +### Fixed +- Fix several potential crashes ([#3465](https://github.com/nomic-ai/gpt4all/pull/3465)) + ## [3.9.0] - 2025-02-04 ### Added @@ -295,6 +300,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Fix several Vulkan resource management issues ([#2694](https://github.com/nomic-ai/gpt4all/pull/2694)) - Fix crash/hang when some models stop generating, by showing special tokens ([#2701](https://github.com/nomic-ai/gpt4all/pull/2701)) +[Unreleased]: https://github.com/nomic-ai/gpt4all/compare/v3.9.0...HEAD [3.9.0]: https://github.com/nomic-ai/gpt4all/compare/v3.8.0...v3.9.0 [3.8.0]: https://github.com/nomic-ai/gpt4all/compare/v3.7.0...v3.8.0 [3.7.0]: https://github.com/nomic-ai/gpt4all/compare/v3.6.1...v3.7.0 diff --git a/gpt4all-chat/src/chatmodel.h b/gpt4all-chat/src/chatmodel.h index a340b0ca..27e6063a 100644 --- a/gpt4all-chat/src/chatmodel.h +++ b/gpt4all-chat/src/chatmodel.h @@ -204,7 +204,8 @@ public: : QObject(nullptr) { moveToThread(parent->thread()); - setParent(parent); + // setParent must be called from the thread the object lives in + QMetaObject::invokeMethod(this, [this, parent]() { this->setParent(parent); }); } // NOTE: System messages are currently never serialized and only *stored* by the local server. diff --git a/gpt4all-chat/src/database.cpp b/gpt4all-chat/src/database.cpp index f118c304..6f9fa5e7 100644 --- a/gpt4all-chat/src/database.cpp +++ b/gpt4all-chat/src/database.cpp @@ -1111,9 +1111,9 @@ class DocumentReader { public: struct Metadata { QString title, author, subject, keywords; }; - static std::unique_ptr fromDocument(const DocumentInfo &info); + static std::unique_ptr fromDocument(DocumentInfo info); - const DocumentInfo &doc () const { return *m_info; } + const DocumentInfo &doc () const { return m_info; } const Metadata &metadata() const { return m_metadata; } const std::optional &word () const { return m_word; } const std::optional &nextWord() { m_word = advance(); return m_word; } @@ -1123,8 +1123,8 @@ public: virtual ~DocumentReader() = default; protected: - explicit DocumentReader(const DocumentInfo &info) - : m_info(&info) {} + explicit DocumentReader(DocumentInfo info) + : m_info(std::move(info)) {} void postInit(Metadata &&metadata = {}) { @@ -1134,9 +1134,9 @@ protected: virtual std::optional advance() = 0; - const DocumentInfo *m_info; - Metadata m_metadata; - std::optional m_word; + DocumentInfo m_info; + Metadata m_metadata; + std::optional m_word; }; namespace { @@ -1144,8 +1144,8 @@ namespace { #ifdef GPT4ALL_USE_QTPDF class PdfDocumentReader final : public DocumentReader { public: - explicit PdfDocumentReader(const DocumentInfo &info) - : DocumentReader(info) + explicit PdfDocumentReader(DocumentInfo info) + : DocumentReader(std::move(info)) { QString path = info.file.canonicalFilePath(); if (m_doc.load(path) != QPdfDocument::Error::None) @@ -1185,8 +1185,8 @@ private: #else class PdfDocumentReader final : public DocumentReader { public: - explicit PdfDocumentReader(const DocumentInfo &info) - : DocumentReader(info) + explicit PdfDocumentReader(DocumentInfo info) + : DocumentReader(std::move(info)) { QString path = info.file.canonicalFilePath(); m_doc = FPDF_LoadDocument(path.toUtf8().constData(), nullptr); @@ -1277,8 +1277,8 @@ private: class WordDocumentReader final : public DocumentReader { public: - explicit WordDocumentReader(const DocumentInfo &info) - : DocumentReader(info) + explicit WordDocumentReader(DocumentInfo info) + : DocumentReader(std::move(info)) , m_doc(info.file.canonicalFilePath().toStdString()) { m_doc.open(); @@ -1370,8 +1370,8 @@ protected: class TxtDocumentReader final : public DocumentReader { public: - explicit TxtDocumentReader(const DocumentInfo &info) - : DocumentReader(info) + explicit TxtDocumentReader(DocumentInfo info) + : DocumentReader(std::move(info)) , m_file(info.file.canonicalFilePath()) { if (!m_file.open(QIODevice::ReadOnly)) @@ -1412,13 +1412,13 @@ protected: } // namespace -std::unique_ptr DocumentReader::fromDocument(const DocumentInfo &doc) +std::unique_ptr DocumentReader::fromDocument(DocumentInfo doc) { if (doc.isPdf()) - return std::make_unique(doc); + return std::make_unique(std::move(doc)); if (doc.isDocx()) - return std::make_unique(doc); - return std::make_unique(doc); + return std::make_unique(std::move(doc)); + return std::make_unique(std::move(doc)); } ChunkStreamer::ChunkStreamer(Database *database) @@ -1426,12 +1426,12 @@ ChunkStreamer::ChunkStreamer(Database *database) ChunkStreamer::~ChunkStreamer() = default; -void ChunkStreamer::setDocument(const DocumentInfo &doc, int documentId, const QString &embeddingModel) +void ChunkStreamer::setDocument(DocumentInfo doc, int documentId, const QString &embeddingModel) { auto docKey = doc.key(); if (!m_docKey || *m_docKey != docKey) { m_docKey = docKey; - m_reader = DocumentReader::fromDocument(doc); + m_reader = DocumentReader::fromDocument(std::move(doc)); m_documentId = documentId; m_embeddingModel = embeddingModel; m_chunk.clear(); @@ -1441,7 +1441,8 @@ void ChunkStreamer::setDocument(const DocumentInfo &doc, int documentId, const Q if (m_database->m_documentIdCache.contains(documentId)) { QSqlQuery q(m_database->m_db); if (!m_database->removeChunksByDocumentId(q, documentId)) - handleDocumentError("ERROR: Cannot remove chunks of document", documentId, doc.file.canonicalPath(), q.lastError()); + handleDocumentError("ERROR: Cannot remove chunks of document", + documentId, m_reader->doc().file.canonicalPath(), q.lastError()); } } } diff --git a/gpt4all-chat/src/database.h b/gpt4all-chat/src/database.h index 30c13942..45924b36 100644 --- a/gpt4all-chat/src/database.h +++ b/gpt4all-chat/src/database.h @@ -171,7 +171,7 @@ public: explicit ChunkStreamer(Database *database); ~ChunkStreamer(); - void setDocument(const DocumentInfo &doc, int documentId, const QString &embeddingModel); + void setDocument(DocumentInfo doc, int documentId, const QString &embeddingModel); std::optional currentDocKey() const; void reset(); diff --git a/gpt4all-chat/src/embllm.cpp b/gpt4all-chat/src/embllm.cpp index 81b1e9e1..7906a56f 100644 --- a/gpt4all-chat/src/embllm.cpp +++ b/gpt4all-chat/src/embllm.cpp @@ -359,8 +359,11 @@ void EmbeddingLLMWorker::handleFinished() if (retrievedData.isValid() && retrievedData.canConvert>()) chunks = retrievedData.value>(); - QVariant response = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); - Q_ASSERT(response.isValid()); + QVariant response; + if (reply->error() != QNetworkReply::NoError) { + response = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); + Q_ASSERT(response.isValid()); + } bool ok; int code = response.toInt(&ok); if (!ok || code != 200) { diff --git a/gpt4all-chat/src/logger.cpp b/gpt4all-chat/src/logger.cpp index 6c730757..ab85d508 100644 --- a/gpt4all-chat/src/logger.cpp +++ b/gpt4all-chat/src/logger.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -62,8 +63,11 @@ void Logger::messageHandler(QtMsgType type, const QMessageLogContext &, const QS } // Get time and date auto timestamp = QDateTime::currentDateTime().toString(); - // Write message + const std::string out = u"[%1] (%2): %3\n"_s.arg(typeString, timestamp, msg).toStdString(); + + // Write message + QMutexLocker locker(&logger->m_mutex); logger->m_file.write(out.c_str()); logger->m_file.flush(); std::cerr << out; diff --git a/gpt4all-chat/src/logger.h b/gpt4all-chat/src/logger.h index 5ca865c0..09d35417 100644 --- a/gpt4all-chat/src/logger.h +++ b/gpt4all-chat/src/logger.h @@ -2,19 +2,23 @@ #define LOGGER_H #include +#include #include #include -class Logger -{ - QFile m_file; - - static void messageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg); - +class Logger { public: + explicit Logger(); + static Logger *globalInstance(); - explicit Logger(); +private: + static void messageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg); + +private: + QFile m_file; + QMutex m_mutex; + friend class MyLogger; }; diff --git a/gpt4all-chat/src/network.cpp b/gpt4all-chat/src/network.cpp index bff38010..70495fbe 100644 --- a/gpt4all-chat/src/network.cpp +++ b/gpt4all-chat/src/network.cpp @@ -242,6 +242,12 @@ void Network::handleJsonUploadFinished() m_activeUploads.removeAll(jsonReply); + if (jsonReply->error() != QNetworkReply::NoError) { + qWarning() << "Request to" << jsonReply->url().toString() << "failed:" << jsonReply->errorString(); + jsonReply->deleteLater(); + return; + } + QVariant response = jsonReply->attribute(QNetworkRequest::HttpStatusCodeAttribute); Q_ASSERT(response.isValid()); bool ok; @@ -449,6 +455,11 @@ void Network::handleIpifyFinished() QNetworkReply *reply = qobject_cast(sender()); if (!reply) return; + if (reply->error() != QNetworkReply::NoError) { + qWarning() << "Request to" << reply->url().toString() << "failed:" << reply->errorString(); + reply->deleteLater(); + return; + } QVariant response = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); Q_ASSERT(response.isValid()); @@ -473,6 +484,11 @@ void Network::handleMixpanelFinished() QNetworkReply *reply = qobject_cast(sender()); if (!reply) return; + if (reply->error() != QNetworkReply::NoError) { + qWarning() << "Request to" << reply->url().toString() << "failed:" << reply->errorString(); + reply->deleteLater(); + return; + } QVariant response = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); Q_ASSERT(response.isValid()); @@ -511,6 +527,11 @@ void Network::handleHealthFinished() QNetworkReply *healthReply = qobject_cast(sender()); if (!healthReply) return; + if (healthReply->error() != QNetworkReply::NoError) { + qWarning() << "Request to" << healthReply->url().toString() << "failed:" << healthReply->errorString(); + healthReply->deleteLater(); + return; + } QVariant response = healthReply->attribute(QNetworkRequest::HttpStatusCodeAttribute); Q_ASSERT(response.isValid()); diff --git a/gpt4all-chat/translations/gpt4all_ro_RO.ts b/gpt4all-chat/translations/gpt4all_ro_RO.ts index 199fa6aa..9b28dd8b 100644 --- a/gpt4all-chat/translations/gpt4all_ro_RO.ts +++ b/gpt4all-chat/translations/gpt4all_ro_RO.ts @@ -498,11 +498,6 @@ enter $BASE_URL introdu $BASE_URL - - - ERROR: $API_KEY is empty. - EROARE: $API_KEY absentă - enter $MODEL_NAME