Async hardening

There are three related changes here to harden our handling of threads
in the footprint async loader.

1) Footprint async loader explicitly aborts any remaining loader threads
on exit.

2) We protect the thread join by a mutex

3) We do not pause during no-wait routines

common/footprint_info.cpp

@@ -147,6 +147,13 @@ FOOTPRINT_ASYNC_LOADER::FOOTPRINT_ASYNC_LOADER() : m_list( nullptr )
 }
 
 
+FOOTPRINT_ASYNC_LOADER::~FOOTPRINT_ASYNC_LOADER()
+{
+    // This is NOP if the load has finished
+    Abort();
+}
+
+
 void FOOTPRINT_ASYNC_LOADER::SetList( FOOTPRINT_LIST* aList )
 {
     m_list = aList;

common/widgets/progress_reporter.cpp

@@ -92,8 +92,6 @@ bool PROGRESS_REPORTER::KeepRefreshing( bool aWait )
     }
     else
     {
-        wxMilliSleep( 20 );
-
         return updateUI();
     }
 }

include/footprint_info.h

@@ -315,6 +315,8 @@ public:
      */
     FOOTPRINT_ASYNC_LOADER();
 
+    ~FOOTPRINT_ASYNC_LOADER();
+
     /**
      * Assign a FOOTPRINT_LIST to the loader. This does not take ownership of
      * the list.

pcbnew/footprint_info_impl.cpp

@@ -39,6 +39,7 @@
 #include <widgets/progress_reporter.h>
 
 #include <thread>
+#include <mutex>
 
 
 void FOOTPRINT_INFO_IMPL::load()
@@ -139,8 +140,8 @@ bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxStri
     {
         if( m_progress_reporter && !m_progress_reporter->KeepRefreshing() )
             m_cancelled = true;
-        else
+
-            wxMilliSleep( 20 );
+        wxMilliSleep( 20 );
     }
 
     if( m_cancelled )
@@ -199,29 +200,36 @@ void FOOTPRINT_LIST_IMPL::StartWorkers( FP_LIB_TABLE* aTable, wxString const* aN
 
 void FOOTPRINT_LIST_IMPL::StopWorkers()
 {
+    std::lock_guard<std::mutex> lock1( m_join );
+
     // 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;
+
+    // If we have cancelled in the middle of a load, clear our timestamp to re-load next time
+    if( m_cancelled )
+        m_list_timestamp = 0;
 }
 
 bool FOOTPRINT_LIST_IMPL::JoinWorkers()
 {
-    for( auto& i : m_threads )
+    {
-        i.join();
+        std::lock_guard<std::mutex> lock1( m_join );
 
-    m_threads.clear();
+        for( auto& i : m_threads )
-    m_queue_in.clear();
+            i.join();
-    m_count_finished.store( 0 );
+
+        m_threads.clear();
+        m_queue_in.clear();
+        m_count_finished.store( 0 );
+    }
 
     size_t total_count = m_queue_out.size();
 
@@ -287,8 +295,8 @@ bool FOOTPRINT_LIST_IMPL::JoinWorkers()
     {
         if( m_progress_reporter && !m_progress_reporter->KeepRefreshing() )
             m_cancelled = true;
-        else
+
-            wxMilliSleep( 20 );
+        wxMilliSleep( 20 );
     }
 
     for( auto& thr : threads )
@@ -325,6 +333,5 @@ FOOTPRINT_LIST_IMPL::FOOTPRINT_LIST_IMPL() :
 
 FOOTPRINT_LIST_IMPL::~FOOTPRINT_LIST_IMPL()
 {
-    for( auto& i : m_threads )
+    StopWorkers();
-        i.join();
 }

pcbnew/footprint_info_impl.h

@@ -65,6 +65,7 @@ class FOOTPRINT_LIST_IMPL : public FOOTPRINT_LIST
     long long                m_list_timestamp;
     PROGRESS_REPORTER*       m_progress_reporter;
     std::atomic_bool         m_cancelled;
+    std::mutex               m_join;
 
     /**
      * Call aFunc, pushing any IO_ERRORs and std::exceptions it throws onto m_errors.

pcbnew/zone_filler.cpp

@@ -147,8 +147,8 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
     {
         if( m_progressReporter )
             m_progressReporter->KeepRefreshing();
-        else
+
-            wxMilliSleep( 20 );
+        wxMilliSleep( 20 );
     }
 
     for( size_t ii = 0; ii < fillWorkers.size(); ++ii )
@@ -242,8 +242,8 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
     {
         if( m_progressReporter )
             m_progressReporter->KeepRefreshing();
-        else
+
-            wxMilliSleep( 10 );
+        wxMilliSleep( 10 );
     }
 
     for( size_t ii = 0; ii < triangulationWorkers.size(); ++ii )