From fb861f9befec51ce17fae46faf15e06c2128a4f8 Mon Sep 17 00:00:00 2001 From: Adam Treat Date: Thu, 19 Dec 2024 08:38:18 -0500 Subject: [PATCH] Fix a crash that can happen when erasing a conversation during active generation. Signed-off-by: Adam Treat --- gpt4all-chat/src/chat.cpp | 7 ++----- gpt4all-chat/src/chatllm.cpp | 20 +++++++++++++++----- gpt4all-chat/src/chatmodel.h | 10 +++++----- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/gpt4all-chat/src/chat.cpp b/gpt4all-chat/src/chat.cpp index 5a2adb097f2d..5780513971d9 100644 --- a/gpt4all-chat/src/chat.cpp +++ b/gpt4all-chat/src/chat.cpp @@ -240,15 +240,12 @@ void Chat::responseStopped(qint64 promptResponseMs) emit responseInProgressChanged(); emit responseStateChanged(); - const int index = m_chatModel->count() - 1; - ChatItem *item = m_chatModel->get(index); - - const QString possibleToolcall = item->toolCallValue(); + const QString possibleToolcall = m_chatModel->possibleToolcall(); ToolCallParser parser; parser.update(possibleToolcall); - if (item->type() == ChatItem::Type::Response && parser.state() == ToolEnums::ParseState::Complete) { + if (parser.state() == ToolEnums::ParseState::Complete) { const QString toolCall = parser.toolCall(); // Regex to remove the formatting around the code diff --git a/gpt4all-chat/src/chatllm.cpp b/gpt4all-chat/src/chatllm.cpp index cc2318d6db01..07877e999a03 100644 --- a/gpt4all-chat/src/chatllm.cpp +++ b/gpt4all-chat/src/chatllm.cpp @@ -740,7 +740,9 @@ std::vector ChatLLM::forkConversation(const QString &prompt) const std::vector conversation; { auto items = m_chatModel->messageItems(); - Q_ASSERT(items.size() >= 2); // should be prompt/response pairs + // It is possible the main thread could have erased the conversation while the llm thread, + // is busy forking the conversatoin but it must have set stop generating first + Q_ASSERT(items.size() >= 2 || m_stopGenerating); // should be prompt/response pairs conversation.reserve(items.size() + 1); conversation.assign(items.begin(), items.end()); } @@ -960,10 +962,18 @@ auto ChatLLM::promptInternal( result.response.append(piece.data(), piece.size()); auto respStr = QString::fromUtf8(result.response); - if (toolCallParser.hasSplit()) - m_chatModel->setResponseValue(toolCallParser.buffer()); - else - m_chatModel->setResponseValue(removeLeadingWhitespace(respStr)); + try { + if (toolCallParser.hasSplit()) + m_chatModel->setResponseValue(toolCallParser.buffer()); + else + m_chatModel->setResponseValue(removeLeadingWhitespace(respStr)); + } catch (const std::exception &e) { + // We have a try/catch here because the main thread might have removed the response from + // the chatmodel by erasing the conversation during the response... the main thread sets + // m_stopGenerating before doing so, but it doesn't wait after that to reset the chatmodel + Q_ASSERT(m_stopGenerating); + return false; + } emit responseChanged(); diff --git a/gpt4all-chat/src/chatmodel.h b/gpt4all-chat/src/chatmodel.h index 3615801d7fa5..0e74924d316e 100644 --- a/gpt4all-chat/src/chatmodel.h +++ b/gpt4all-chat/src/chatmodel.h @@ -305,10 +305,10 @@ class ChatItem : public QObject return items; } - QString toolCallValue() const + QString possibleToolCall() const { if (!subItems.empty()) - return subItems.back()->toolCallValue(); + return subItems.back()->possibleToolCall(); if (type() == Type::ToolCall) return value; else @@ -735,11 +735,11 @@ class ChatModel : public QAbstractListModel emit hasErrorChanged(false); } - Q_INVOKABLE ChatItem *get(int index) + Q_INVOKABLE QString possibleToolcall() const { QMutexLocker locker(&m_mutex); - if (index < 0 || index >= m_chatItems.size()) return nullptr; - return m_chatItems.at(index); + if (m_chatItems.empty()) return QString(); + return m_chatItems.back()->possibleToolCall(); } Q_INVOKABLE void updateCurrentResponse(int index, bool b)