Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc fixes for undefined behavior, crashes, and build failure #3465

Merged
merged 7 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions gpt4all-backend/src/llmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,14 @@ const std::vector<LLModel::Implementation> &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;
Expand Down
6 changes: 6 additions & 0 deletions gpt4all-chat/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion gpt4all-chat/src/chatmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ class ChatItem : public QObject
: 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.
Expand Down
45 changes: 23 additions & 22 deletions gpt4all-chat/src/database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1111,9 +1111,9 @@ class DocumentReader {
public:
struct Metadata { QString title, author, subject, keywords; };

static std::unique_ptr<DocumentReader> fromDocument(const DocumentInfo &info);
static std::unique_ptr<DocumentReader> 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<QString> &word () const { return m_word; }
const std::optional<QString> &nextWord() { m_word = advance(); return m_word; }
Expand All @@ -1123,8 +1123,8 @@ class DocumentReader {
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 = {})
{
Expand All @@ -1134,18 +1134,18 @@ class DocumentReader {

virtual std::optional<QString> advance() = 0;

const DocumentInfo *m_info;
Metadata m_metadata;
std::optional<QString> m_word;
DocumentInfo m_info;
Metadata m_metadata;
std::optional<QString> m_word;
};

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)
Expand Down Expand Up @@ -1185,8 +1185,8 @@ class PdfDocumentReader final : public DocumentReader {
#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);
Expand Down Expand Up @@ -1277,8 +1277,8 @@ class PdfDocumentReader final : public DocumentReader {

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();
Expand Down Expand Up @@ -1370,8 +1370,8 @@ class WordDocumentReader final : public DocumentReader {

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))
Expand Down Expand Up @@ -1412,26 +1412,26 @@ class TxtDocumentReader final : public DocumentReader {

} // namespace

std::unique_ptr<DocumentReader> DocumentReader::fromDocument(const DocumentInfo &doc)
std::unique_ptr<DocumentReader> DocumentReader::fromDocument(DocumentInfo doc)
{
if (doc.isPdf())
return std::make_unique<PdfDocumentReader>(doc);
return std::make_unique<PdfDocumentReader>(std::move(doc));
if (doc.isDocx())
return std::make_unique<WordDocumentReader>(doc);
return std::make_unique<TxtDocumentReader>(doc);
return std::make_unique<WordDocumentReader>(std::move(doc));
return std::make_unique<TxtDocumentReader>(std::move(doc));
}

ChunkStreamer::ChunkStreamer(Database *database)
: m_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();
Expand All @@ -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());
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion gpt4all-chat/src/database.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ class ChunkStreamer {
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<DocumentInfo::key_type> currentDocKey() const;
void reset();

Expand Down
7 changes: 5 additions & 2 deletions gpt4all-chat/src/embllm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,11 @@ void EmbeddingLLMWorker::handleFinished()
if (retrievedData.isValid() && retrievedData.canConvert<QVector<EmbeddingChunk>>())
chunks = retrievedData.value<QVector<EmbeddingChunk>>();

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) {
Expand Down
6 changes: 5 additions & 1 deletion gpt4all-chat/src/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <QDebug>
#include <QGlobalStatic>
#include <QIODevice>
#include <QMutexLocker>
#include <QStandardPaths>

#include <cstdio>
Expand Down Expand Up @@ -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;
Expand Down
16 changes: 10 additions & 6 deletions gpt4all-chat/src/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,23 @@
#define LOGGER_H

#include <QFile>
#include <QMutex>
#include <QString>
#include <QtLogging>

class Logger
{
QFile m_file;
class Logger {
public:
explicit Logger();

static Logger *globalInstance();

private:
static void messageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg);

public:
static Logger *globalInstance();
private:
QFile m_file;
QMutex m_mutex;

explicit Logger();
friend class MyLogger;
};

Expand Down
21 changes: 21 additions & 0 deletions gpt4all-chat/src/network.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -449,6 +455,11 @@ void Network::handleIpifyFinished()
QNetworkReply *reply = qobject_cast<QNetworkReply *>(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());
Expand All @@ -473,6 +484,11 @@ void Network::handleMixpanelFinished()
QNetworkReply *reply = qobject_cast<QNetworkReply *>(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());
Expand Down Expand Up @@ -511,6 +527,11 @@ void Network::handleHealthFinished()
QNetworkReply *healthReply = qobject_cast<QNetworkReply *>(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());
Expand Down
5 changes: 0 additions & 5 deletions gpt4all-chat/translations/gpt4all_ro_RO.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,11 +498,6 @@
<source>enter $BASE_URL</source>
<translation>introdu $BASE_URL</translation>
</message>
<message>
<location filename="../qml/AddHFModelView.qml" line="600"/>
<source>ERROR: $API_KEY is empty.</source>
<translation>EROARE: $API_KEY absentă</translation>
</message>
<message>
<location filename="../qml/AddHFModelView.qml" line="606"/>
<source>enter $MODEL_NAME</source>
Expand Down