From 8af4cf88a0bd1e21e3a2d08e8ea4c4c5464d955f Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Mon, 4 Jan 2021 09:12:18 -0800
Subject: [PATCH] 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
---
 .gitignore                        |  5 +++--
 common/lib_tree_model_adapter.cpp | 20 ++++++++++++--------
 common/widgets/lib_tree.cpp       |  2 +-
 include/lib_tree_model_adapter.h  |  3 ++-
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/.gitignore b/.gitignore
index 89356377b4..7969e77860 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,6 @@
-boost_root
 .downloads-by-cmake
+.gdb_history
+boost_root
 Build*
 build*
 common/fp_lib_table_keywords.cpp
@@ -114,4 +115,4 @@ CMakeSettings.json
 # Translations
 *.mo
 i18n_status.svg
-i18n_status.csv
\ No newline at end of file
+i18n_status.csv
diff --git a/common/lib_tree_model_adapter.cpp b/common/lib_tree_model_adapter.cpp
index 7d31bf76d1..e0be4dd411 100644
--- a/common/lib_tree_model_adapter.cpp
+++ b/common/lib_tree_model_adapter.cpp
@@ -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 );
 
-        // 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
         // run a wxdataview_selection_changed_callback() on a row that has been deleted.
         // https://bugs.launchpad.net/kicad/+bug/1756255
         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
         // 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.
diff --git a/common/widgets/lib_tree.cpp b/common/widgets/lib_tree.cpp
index 550875861b..8e8f3ddc4c 100644
--- a/common/widgets/lib_tree.cpp
+++ b/common/widgets/lib_tree.cpp
@@ -203,7 +203,7 @@ void LIB_TREE::Regenerate( bool aKeepState )
         current = getState();
 
     wxString filter = m_query_ctrl->GetValue();
-    m_adapter->UpdateSearchString( filter );
+    m_adapter->UpdateSearchString( filter, aKeepState );
     postPreselectEvent();
 
     // Restore the state
diff --git a/include/lib_tree_model_adapter.h b/include/lib_tree_model_adapter.h
index 6fe93a4a85..9ef3ee7db9 100644
--- a/include/lib_tree_model_adapter.h
+++ b/include/lib_tree_model_adapter.h
@@ -179,8 +179,9 @@ public:
      * Set the search string provided by the user.
      *
      * @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