From cf2a07559d047ba56f2ba3b0e3a392a53a499ccb Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@ucdavis.edu>
Date: Wed, 23 May 2018 08:52:48 -0700
Subject: [PATCH] Safely release thread memory

When starting async processes, we need to have a way to stop the process
before releasing memory.  Descoping FOOTPRINT_ASYNC_LOADER while the
threads were still running could cause crashes depending on the memory
structure.  To avoid this we define clear procedures for exiting a
running async processes and call these when exiting.

Fixes: lp:1772909
* https://bugs.launchpad.net/kicad/+bug/1772909
---
 common/footprint_info.cpp      | 10 +++++++++
 include/footprint_info.h       | 10 +++++++++
 pcbnew/footprint_info_impl.cpp | 37 +++++++++++++++++++++++-----------
 pcbnew/footprint_info_impl.h   |  2 ++
 4 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/common/footprint_info.cpp b/common/footprint_info.cpp
index b122e7112b..c4db568d64 100644
--- a/common/footprint_info.cpp
+++ b/common/footprint_info.cpp
@@ -181,6 +181,16 @@ bool FOOTPRINT_ASYNC_LOADER::Join()
 }
 
 
+void FOOTPRINT_ASYNC_LOADER::Abort()
+{
+    if( m_list )
+    {
+        m_list->StopWorkers();
+        m_list = nullptr;
+    }
+}
+
+
 void FOOTPRINT_ASYNC_LOADER::SetCompletionCallback( std::function<void()> aCallback )
 {
     m_completion_cb = std::move(aCallback);
diff --git a/include/footprint_info.h b/include/footprint_info.h
index 673b7277d8..e0d36528f4 100644
--- a/include/footprint_info.h
+++ b/include/footprint_info.h
@@ -284,6 +284,11 @@ protected:
      * Join worker threads. Part of the FOOTPRINT_ASYNC_LOADER implementation.
      */
     virtual bool JoinWorkers() = 0;
+
+    /**
+     * Stop worker threads. Part of the FOOTPRINT_ASYNC_LOADER implementation.
+     */
+    virtual void StopWorkers() = 0;
 };
 
 
@@ -342,6 +347,11 @@ public:
      */
     bool Join();
 
+    /**
+     * Safely stop the current process.
+     */
+    void Abort();
+
     /**
      * Set a callback to receive notice when loading is complete.
      *
diff --git a/pcbnew/footprint_info_impl.cpp b/pcbnew/footprint_info_impl.cpp
index b7e44d776b..606a16f015 100644
--- a/pcbnew/footprint_info_impl.cpp
+++ b/pcbnew/footprint_info_impl.cpp
@@ -143,27 +143,23 @@ bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxStri
             wxMilliSleep( 20 );
     }
 
-    if( m_progress_reporter )
-        m_progress_reporter->AdvancePhase();
-
-    if( !m_cancelled )
+    if( m_cancelled )
+    {
+        loader.Abort();
+    }
+    else
     {
         if( m_progress_reporter )
         {
+            m_progress_reporter->AdvancePhase();
             m_progress_reporter->SetMaxProgress( m_queue_out.size() );
             m_progress_reporter->Report( _( "Loading Footprints" ) );
         }
 
         loader.Join();
-    }
 
-    if( m_progress_reporter )
-        m_progress_reporter->AdvancePhase();
-
-    if( m_cancelled )
-    {
-        m_errors.move_push( std::make_unique<IO_ERROR>
-                    ( _( "Loading incomplete; cancelled by user." ), nullptr, nullptr, 0 ) );
+        if( m_progress_reporter )
+            m_progress_reporter->AdvancePhase();
     }
 
     m_progress_reporter = nullptr;
@@ -203,6 +199,23 @@ void FOOTPRINT_LIST_IMPL::StartWorkers( FP_LIB_TABLE* aTable, wxString const* aN
     }
 }
 
+void FOOTPRINT_LIST_IMPL::StopWorkers()
+{
+    // To safely stop our workers, we set the cancellation flag (they will each
+    // exit on their next safe loop location when this is set).  Then we need to wait
+    // for all threads to finish as closing the implementation will free the queues
+    // that the threads write to.
+    m_cancelled = true;
+
+    for( auto& i : m_threads )
+        i.join();
+
+    m_threads.clear();
+    m_queue_in.clear();
+    m_count_finished.store( 0 );
+    m_list_timestamp = 0;
+}
+
 bool FOOTPRINT_LIST_IMPL::JoinWorkers()
 {
     for( auto& i : m_threads )
diff --git a/pcbnew/footprint_info_impl.h b/pcbnew/footprint_info_impl.h
index a2e06a89c6..fb38cc3024 100644
--- a/pcbnew/footprint_info_impl.h
+++ b/pcbnew/footprint_info_impl.h
@@ -78,6 +78,8 @@ protected:
                        FOOTPRINT_ASYNC_LOADER* aLoader, unsigned aNThreads ) override;
     bool JoinWorkers() override;
 
+    void StopWorkers() override;
+
     /**
      * Function loader_job
      * loads footprints from m_queue_in.