Fix crash when deleting symbol in GTK

So many things can go wrong with this control in GTK.  We have to
collapse the tree when updating the search string to avoid a crash when
referencing a child object but collapsing the tree will iterate over
elements and crash when we have deleted a symbol.

The temporary fix for this nonsense is to carefully order the calls.
We only need to collapse the search tree if we are not keeping our state
(in other words if we are fully re-building the tree)

Fixes https://gitlab.com/kicad/code/kicad/issues/6910

(cherry picked from commit 8af4cf88a0)
This commit is contained in:
Seth Hillbrand 2021-01-04 09:12:18 -08:00
parent 1f6ff4a519
commit 20909d4948
4 changed files with 34 additions and 11 deletions

20
.gitignore vendored
View File

@ -1,5 +1,6 @@
boost_root
.downloads-by-cmake .downloads-by-cmake
.gdb_history
boost_root
Build* Build*
build* build*
common/fp_lib_table_keywords.cpp common/fp_lib_table_keywords.cpp
@ -95,3 +96,20 @@ pcbnew/Info.plist
.cproject .cproject
.pydevproject .pydevproject
__pycache__ __pycache__
# Visual Studio
.vs/
.editorconfig
CMakeSettings.json
# Sublime Text
*.sublime-*
# KDevelop
.kdev4/
*.kdev4
# Translations
*.mo
i18n_status.svg
i18n_status.csv

View File

@ -136,23 +136,27 @@ void LIB_TREE_MODEL_ADAPTER::DoAddLibrary( wxString const& aNodeName, wxString c
} }
void LIB_TREE_MODEL_ADAPTER::UpdateSearchString( wxString const& aSearch ) void LIB_TREE_MODEL_ADAPTER::UpdateSearchString( wxString const& aSearch, bool aState )
{ {
{ {
wxWindowUpdateLocker updateLock( m_widget ); wxWindowUpdateLocker updateLock( m_widget );
// This collapse is required before the call to "Freeze()" below. Once Freeze()
// is called, GetParent() will return nullptr. While this works for some calls, it
// segfaults when we have our first library expanded.
// The tree will be expanded again below when we get our matches
if( !aSearch.IsNull() && m_tree.Children.size() )
m_widget->Collapse( wxDataViewItem( &*m_tree.Children[0] ) );
// Even with the updateLock, wxWidgets sometimes ties its knickers in a knot trying to // Even with the updateLock, wxWidgets sometimes ties its knickers in a knot trying to
// run a wxdataview_selection_changed_callback() on a row that has been deleted. // run a wxdataview_selection_changed_callback() on a row that has been deleted.
// https://bugs.launchpad.net/kicad/+bug/1756255 // https://bugs.launchpad.net/kicad/+bug/1756255
m_widget->UnselectAll(); m_widget->UnselectAll();
// This collapse is required before the call to "Freeze()" below. Once Freeze()
// is called, GetParent() will return nullptr. While this works for some calls, it
// segfaults when we have our first library expanded.
// The tree will be expanded again below when we get our matches
//
// Also note that this cannot happen when we have deleted a symbol as GTK will also
// iterate over the tree in this case and find a symbol that has an invalid link
// and crash https://gitlab.com/kicad/code/kicad/-/issues/6910
if( !aState && !aSearch.IsNull() && m_tree.Children.size() )
m_widget->Collapse( wxDataViewItem( &*m_tree.Children[0] ) );
// DO NOT REMOVE THE FREEZE/THAW. This freeze/thaw is a flag for this model adapter // DO NOT REMOVE THE FREEZE/THAW. This freeze/thaw is a flag for this model adapter
// that tells it when it shouldn't trust any of the data in the model. When set, it will // that tells it when it shouldn't trust any of the data in the model. When set, it will
// not return invalid data to the UI, since this invalid data can cause crashes. // not return invalid data to the UI, since this invalid data can cause crashes.

View File

@ -171,8 +171,9 @@ public:
* Set the search string provided by the user. * Set the search string provided by the user.
* *
* @param aSearch full, unprocessed search text * @param aSearch full, unprocessed search text
* @param aState Are we keeping the selection/search state?
*/ */
void UpdateSearchString( wxString const& aSearch ); void UpdateSearchString( wxString const& aSearch, bool aState );
/** /**
* Attach to a wxDataViewCtrl and initialize it. This will set up columns * Attach to a wxDataViewCtrl and initialize it. This will set up columns

View File

@ -196,7 +196,7 @@ void LIB_TREE::Regenerate( bool aKeepState )
m_unfilteredState = getState(); m_unfilteredState = getState();
wxString filter = m_query_ctrl->GetValue(); wxString filter = m_query_ctrl->GetValue();
m_adapter->UpdateSearchString( filter ); m_adapter->UpdateSearchString( filter, aKeepState );
postPreselectEvent(); postPreselectEvent();
// Restore the state // Restore the state