From f2a70f958916bec065cb0ee1b098c39fc4e899d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Fri, 6 Mar 2026 00:53:05 -0300 Subject: [PATCH] This should fix two crashes: * A crash during debugging that happens due to invalid memory access (UIAbstractView + VariablesModel changes). * A crash on ChatUI when closing a chat that was about to receive a server response. --- include/eepp/ui/abstract/uiabstractview.hpp | 2 + src/eepp/ui/abstract/uiabstracttableview.cpp | 5 +- src/eepp/ui/abstract/uiabstractview.cpp | 14 ++-- .../ecode/plugins/aiassistant/chatui.cpp | 67 +++++++++++-------- .../aiassistant/llmchatcompletionrequest.cpp | 3 + .../debugger/models/variablesmodel.cpp | 20 +++++- .../debugger/models/variablesmodel.hpp | 2 + 7 files changed, 76 insertions(+), 37 deletions(-) diff --git a/include/eepp/ui/abstract/uiabstractview.hpp b/include/eepp/ui/abstract/uiabstractview.hpp index 6267d6f4a..a93534d54 100644 --- a/include/eepp/ui/abstract/uiabstractview.hpp +++ b/include/eepp/ui/abstract/uiabstractview.hpp @@ -1,6 +1,7 @@ #ifndef EE_UI_UIABSTRACTVIEW_HPP #define EE_UI_UIABSTRACTVIEW_HPP +#include #include #include #include @@ -150,6 +151,7 @@ class EE_API UIAbstractView : public UIScrollableWidget { std::vector mEditShortcuts{ { KEY_F2 } }; SelectionType mSelectionType{ SelectionType::Row }; SelectionKind mSelectionKind{ SelectionKind::Single }; + std::atomic mPendingUpdateFlags{ 0 }; virtual void editingWidgetDidChange( const ModelIndex& ) {} }; diff --git a/src/eepp/ui/abstract/uiabstracttableview.cpp b/src/eepp/ui/abstract/uiabstracttableview.cpp index 930114f4d..8f69a8137 100644 --- a/src/eepp/ui/abstract/uiabstracttableview.cpp +++ b/src/eepp/ui/abstract/uiabstracttableview.cpp @@ -118,11 +118,12 @@ size_t UIAbstractTableView::getItemCount() const { } void UIAbstractTableView::onModelUpdate( unsigned flags ) { + mPendingUpdateFlags.fetch_or( flags ); if ( !Engine::instance()->isMainThread() ) { removeActionsByTag( onModelUpdateTag ); runOnMainThread( - [this, flags] { - modelUpdate( flags ); + [this] { + modelUpdate( mPendingUpdateFlags.exchange( 0 ) ); createOrUpdateColumns( true ); }, Time::Zero, onModelUpdateTag ); diff --git a/src/eepp/ui/abstract/uiabstractview.cpp b/src/eepp/ui/abstract/uiabstractview.cpp index d103ce953..6514760b5 100644 --- a/src/eepp/ui/abstract/uiabstractview.cpp +++ b/src/eepp/ui/abstract/uiabstractview.cpp @@ -4,6 +4,8 @@ namespace EE { namespace UI { namespace Abstract { +static constexpr String::HashType OnModelUpdateTag = String::hash( "onModelUpdate" ); + UIAbstractView::UIAbstractView( const std::string& tag ) : UIScrollableWidget( tag ), mSelection( this ) {} @@ -126,7 +128,8 @@ void UIAbstractView::setModel( const std::shared_ptr& model ) { } void UIAbstractView::modelUpdate( unsigned flags ) { - if ( !getModel() || ( flags & Model::InvalidateAllIndexes ) ) { + mPendingUpdateFlags = 0; + if ( !getModel() || ( flags & Model::UpdateFlag::InvalidateAllIndexes ) ) { getSelection().clear(); } else { getSelection().removeAllMatching( @@ -135,12 +138,13 @@ void UIAbstractView::modelUpdate( unsigned flags ) { } void UIAbstractView::onModelUpdate( unsigned flags ) { + mPendingUpdateFlags.fetch_or( flags ); if ( !Engine::instance()->isMainThread() ) { - static constexpr String::HashType tag = String::hash( "onModelUpdate" ); - removeActionsByTag( tag ); - runOnMainThread( [this, flags] { modelUpdate( flags ); }, Time::Zero, tag ); + removeActionsByTag( OnModelUpdateTag ); + runOnMainThread( [this] { modelUpdate( mPendingUpdateFlags.exchange( 0 ) ); }, Time::Zero, + OnModelUpdateTag ); } else { - modelUpdate( flags ); + modelUpdate( mPendingUpdateFlags.exchange( 0 ) ); } } diff --git a/src/tools/ecode/plugins/aiassistant/chatui.cpp b/src/tools/ecode/plugins/aiassistant/chatui.cpp index 08da78cc4..454567729 100644 --- a/src/tools/ecode/plugins/aiassistant/chatui.cpp +++ b/src/tools/ecode/plugins/aiassistant/chatui.cpp @@ -595,6 +595,16 @@ LLMChatUI::LLMChatUI( PluginManager* manager ) : } LLMChatUI::~LLMChatUI() { + if ( mRequest ) { + mRequest->cancelCb = nullptr; + mRequest->doneCb = nullptr; + mRequest->streamedResponseCb = nullptr; + } + if ( mSummaryRequest ) { + mSummaryRequest->cancelCb = nullptr; + mSummaryRequest->doneCb = nullptr; + mSummaryRequest->streamedResponseCb = nullptr; + } if ( getPlugin() ) { AIAssistantPlugin::AIAssistantConfig config; config.partition = getSplitter()->getSplitPartition(); @@ -1168,28 +1178,27 @@ void LLMChatUI::generateChatName( bool isRenaming ) { mSummaryRequest->doneCb = [this, isRenaming]( const LLMChatCompletionRequest& req, Http::Response& response ) { auto status = response.getStatus(); - String oldSummary = std::move( mSummary ); + runOnMainThread( [this, isRenaming, status, responseText = req.getResponse()] { + String oldSummary = std::move( mSummary ); - if ( status == Http::Response::Ok ) { - mSummary = String::trim( req.getResponse() ); - String::trimInPlace( mSummary, '\n' ); - String::trimInPlace( mSummary, ' ' ); - String::trimInPlace( mSummary, '"' ); - } else { - // TODO: Implement generating a summary based on the user prompt (take the - // first few words) - mSummary = i18n( "untitled_conversation", "Untitled Conversation" ); - } + if ( status == Http::Response::Ok ) { + mSummary = String::trim( responseText ); + String::trimInPlace( mSummary, '\n' ); + String::trimInPlace( mSummary, ' ' ); + String::trimInPlace( mSummary, '"' ); + } else { + // TODO: Implement generating a summary based on the user prompt (take the + // first few words) + mSummary = i18n( "untitled_conversation", "Untitled Conversation" ); + } - if ( isRenaming ) { - String newSummary = std::move( mSummary ); - mSummary = std::move( oldSummary ); - runOnMainThread( - [this, newSummary = std::move( newSummary )] { renameChat( newSummary ); } ); - } else - saveChat(); + if ( isRenaming ) { + String newSummary = std::move( mSummary ); + mSummary = std::move( oldSummary ); + renameChat( newSummary ); + } else + saveChat(); - runOnMainThread( [this] { updateTabTitle(); mSummaryRequest.reset(); } ); @@ -1258,15 +1267,17 @@ void LLMChatUI::doRequest() { }; mRequest->cancelCb = [this, thinking, thinkingID, editor]( const LLMChatCompletionRequest& ) { - thinking->removeActionsByTag( thinkingID ); - thinking->setVisible( false ); - mChatStop->setVisible( false )->setEnabled( false ); - mChatRun->setVisible( true )->setEnabled( true ); - toggleEnableChats( true ); - editor->setEnabled( true ); - if ( editor->hasFocus() ) - mChatInput->setFocus(); - removeLastChat(); + runOnMainThread( [this, thinking, thinkingID, editor] { + thinking->removeActionsByTag( thinkingID ); + thinking->setVisible( false ); + mChatStop->setVisible( false )->setEnabled( false ); + mChatRun->setVisible( true )->setEnabled( true ); + toggleEnableChats( true ); + editor->setEnabled( true ); + if ( editor->hasFocus() ) + mChatInput->setFocus(); + removeLastChat(); + } ); }; mRequest->doneCb = diff --git a/src/tools/ecode/plugins/aiassistant/llmchatcompletionrequest.cpp b/src/tools/ecode/plugins/aiassistant/llmchatcompletionrequest.cpp index b11d92dfe..ece7d6fba 100644 --- a/src/tools/ecode/plugins/aiassistant/llmchatcompletionrequest.cpp +++ b/src/tools/ecode/plugins/aiassistant/llmchatcompletionrequest.cpp @@ -120,6 +120,9 @@ LLMChatCompletionRequest::LLMChatCompletionRequest( const std::string& uri, cons } LLMChatCompletionRequest::~LLMChatCompletionRequest() { + cancelCb = nullptr; + doneCb = nullptr; + streamedResponseCb = nullptr; cancel(); } diff --git a/src/tools/ecode/plugins/debugger/models/variablesmodel.cpp b/src/tools/ecode/plugins/debugger/models/variablesmodel.cpp index afaa98921..3bea010cc 100644 --- a/src/tools/ecode/plugins/debugger/models/variablesmodel.cpp +++ b/src/tools/ecode/plugins/debugger/models/variablesmodel.cpp @@ -2,6 +2,7 @@ #include "../debuggerclient.hpp" #include #include +#include namespace ecode { @@ -73,6 +74,7 @@ VariablesModel::VariablesModel( ModelVariableNode::NodePtr rootNode, UISceneNode mRootNode( rootNode ), mSceneNode( sceneNode ) {} ModelIndex VariablesModel::index( int row, int column, const ModelIndex& parent ) const { + checkQueuedClear( parent ); if ( !mRootNode ) return ModelIndex(); @@ -115,6 +117,7 @@ ModelIndex VariablesModel::parentIndex( const ModelIndex& index ) const { } size_t VariablesModel::rowCount( const ModelIndex& index ) const { + checkQueuedClear( index ); ModelVariableNode* parentNode = index.isValid() ? static_cast( index.internalData() ) : mRootNode.get(); @@ -122,6 +125,7 @@ size_t VariablesModel::rowCount( const ModelIndex& index ) const { } bool VariablesModel::hasChildren( const ModelIndex& index ) const { + checkQueuedClear( index ); if ( !index.isValid() ) return !mRootNode->children.empty(); ModelVariableNode* node = static_cast( index.internalData() ); @@ -165,9 +169,21 @@ Variant VariablesModel::data( const ModelIndex& index, ModelRole role ) const { return EMPTY; } +void VariablesModel::checkQueuedClear( const ModelIndex& index ) const { + if ( !index.isValid() && mQueuedClear && Engine::instance()->isMainThread() ) { + mChildMap.clear(); + mQueuedClear = false; + } +} + void VariablesModel::invalidate( unsigned int flags ) { if ( flags & Model::UpdateFlag::InvalidateAllIndexes ) { - mChildMap.clear(); + if ( Engine::instance()->isMainThread() ) { + mChildMap.clear(); + mQueuedClear = false; + } else { + mQueuedClear = true; + } } Model::invalidate( flags ); } @@ -246,7 +262,7 @@ void VariablesHolder::upsertRootChild( Variable&& var ) { auto newChild = std::make_shared( std::move( var ), mRootNode ); mNodeMap[newChild->var.variablesReference] = newChild; mRootNode->children[i] = std::move( newChild ); - mModel->invalidate( Model::UpdateFlag::DontInvalidateIndexes ); + mModel->invalidate( Model::UpdateFlag::InvalidateAllIndexes ); return; } } diff --git a/src/tools/ecode/plugins/debugger/models/variablesmodel.hpp b/src/tools/ecode/plugins/debugger/models/variablesmodel.hpp index eeab3584d..2b81c6555 100644 --- a/src/tools/ecode/plugins/debugger/models/variablesmodel.hpp +++ b/src/tools/ecode/plugins/debugger/models/variablesmodel.hpp @@ -125,7 +125,9 @@ class VariablesModel : public Model { ModelVariableNode::NodePtr mRootNode; mutable std::unordered_map mChildMap; UISceneNode* mSceneNode; + mutable bool mQueuedClear{ false }; + void checkQueuedClear( const ModelIndex& index ) const; }; class VariablesHolder {