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
This commit is contained in:
Seth Hillbrand 2021-01-04 09:12:18 -08:00
parent 51f40a3f2b
commit 8af4cf88a0
4 changed files with 18 additions and 12 deletions

5
.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
@ -114,4 +115,4 @@ CMakeSettings.json
# Translations # Translations
*.mo *.mo
i18n_status.svg i18n_status.svg
i18n_status.csv i18n_status.csv

View File

@ -187,23 +187,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.m_Children.size() )
m_widget->Collapse( wxDataViewItem( &*m_tree.m_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.m_Children.size() )
m_widget->Collapse( wxDataViewItem( &*m_tree.m_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

@ -203,7 +203,7 @@ void LIB_TREE::Regenerate( bool aKeepState )
current = getState(); current = 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

View File

@ -179,8 +179,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 if true, we are keeping the state and so we shouldn't collapse the tree
*/ */
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