From 20909d4948c97d1c4f58338adce26c99f64279b8 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

(cherry picked from commit 8af4cf88a0bd1e21e3a2d08e8ea4c4c5464d955f)
---
 .gitignore                        | 20 +++++++++++++++++++-
 common/lib_tree_model_adapter.cpp | 20 ++++++++++++--------
 common/lib_tree_model_adapter.h   |  3 ++-
 common/widgets/lib_tree.cpp       |  2 +-
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/.gitignore b/.gitignore
index 7a4c04491c..37337e8844 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
@@ -95,3 +96,20 @@ pcbnew/Info.plist
 .cproject
 .pydevproject
 __pycache__
+
+# Visual Studio
+.vs/
+.editorconfig
+CMakeSettings.json
+
+# Sublime Text
+*.sublime-*
+
+# KDevelop
+.kdev4/
+*.kdev4
+
+# Translations
+*.mo
+i18n_status.svg
+i18n_status.csv
diff --git a/common/lib_tree_model_adapter.cpp b/common/lib_tree_model_adapter.cpp
index bb22210a68..92432915c2 100644
--- a/common/lib_tree_model_adapter.cpp
+++ b/common/lib_tree_model_adapter.cpp
@@ -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 );
 
-        // 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
         // 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.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
         // 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/lib_tree_model_adapter.h b/common/lib_tree_model_adapter.h
index 1446cd70ce..831a2ec6ff 100644
--- a/common/lib_tree_model_adapter.h
+++ b/common/lib_tree_model_adapter.h
@@ -171,8 +171,9 @@ public:
      * Set the search string provided by the user.
      *
      * @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
diff --git a/common/widgets/lib_tree.cpp b/common/widgets/lib_tree.cpp
index 0ffbd43b2e..3bc288a671 100644
--- a/common/widgets/lib_tree.cpp
+++ b/common/widgets/lib_tree.cpp
@@ -196,7 +196,7 @@ void LIB_TREE::Regenerate( bool aKeepState )
         m_unfilteredState = getState();
 
     wxString filter = m_query_ctrl->GetValue();
-    m_adapter->UpdateSearchString( filter );
+    m_adapter->UpdateSearchString( filter, aKeepState );
     postPreselectEvent();
 
     // Restore the state