diff --git a/common/background_jobs_monitor.cpp b/common/background_jobs_monitor.cpp index 4c925fb036..e7c9e33151 100644 --- a/common/background_jobs_monitor.cpp +++ b/common/background_jobs_monitor.cpp @@ -40,7 +40,7 @@ class BACKGROUND_JOB_PANEL : public wxPanel { public: - BACKGROUND_JOB_PANEL( wxWindow* aParent, BACKGROUND_JOB* aJob ) : + BACKGROUND_JOB_PANEL( wxWindow* aParent, std::shared_ptr<BACKGROUND_JOB> aJob ) : wxPanel( aParent, wxID_ANY, wxDefaultPosition, wxSize( -1, 75 ), wxBORDER_SIMPLE ), m_job( aJob ) @@ -86,7 +86,7 @@ private: wxGauge* m_progress; wxStaticText* m_stName; wxStaticText* m_stStatus; - BACKGROUND_JOB* m_job; + std::shared_ptr<BACKGROUND_JOB> m_job; }; @@ -127,7 +127,7 @@ public: } - void Add( BACKGROUND_JOB* aJob ) + void Add( std::shared_ptr<BACKGROUND_JOB> aJob ) { BACKGROUND_JOB_PANEL* panel = new BACKGROUND_JOB_PANEL( m_scrolledWindow, aJob ); m_contentSizer->Add( panel, 0, wxEXPAND | wxALL, 2 ); @@ -141,7 +141,7 @@ public: } - void Remove( BACKGROUND_JOB* aJob ) + void Remove( std::shared_ptr<BACKGROUND_JOB> aJob ) { auto it = m_jobPanels.find( aJob ); if( it != m_jobPanels.end() ) @@ -154,7 +154,7 @@ public: } } - void UpdateJob( BACKGROUND_JOB* aJob ) + void UpdateJob( std::shared_ptr<BACKGROUND_JOB> aJob ) { auto it = m_jobPanels.find( aJob ); if( it != m_jobPanels.end() ) @@ -167,12 +167,12 @@ public: private: wxScrolledWindow* m_scrolledWindow; wxBoxSizer* m_contentSizer; - std::unordered_map<BACKGROUND_JOB*, BACKGROUND_JOB_PANEL*> m_jobPanels; + std::unordered_map<std::shared_ptr<BACKGROUND_JOB>, BACKGROUND_JOB_PANEL*> m_jobPanels; }; BACKGROUND_JOB_REPORTER::BACKGROUND_JOB_REPORTER( BACKGROUND_JOBS_MONITOR* aMonitor, - BACKGROUND_JOB* aJob ) : + std::shared_ptr<BACKGROUND_JOB> aJob ) : PROGRESS_REPORTER_BASE( 1 ), m_monitor( aMonitor ), m_job( aJob ) { @@ -215,13 +215,14 @@ BACKGROUND_JOBS_MONITOR::BACKGROUND_JOBS_MONITOR() : m_jobListDialog( nullptr ) } -BACKGROUND_JOB* BACKGROUND_JOBS_MONITOR::Create( const wxString& aName ) +std::shared_ptr<BACKGROUND_JOB> BACKGROUND_JOBS_MONITOR::Create( const wxString& aName ) { - BACKGROUND_JOB* job = new BACKGROUND_JOB(); + std::shared_ptr<BACKGROUND_JOB> job = std::make_shared<BACKGROUND_JOB>(); job->m_name = aName; job->m_reporter = std::make_shared<BACKGROUND_JOB_REPORTER>( this, job ); + std::lock_guard<std::shared_mutex> lock( m_mutex ); m_jobs.push_back( job ); if( m_shownDialogs.size() > 0 ) @@ -229,7 +230,11 @@ BACKGROUND_JOB* BACKGROUND_JOBS_MONITOR::Create( const wxString& aName ) // update dialogs for( BACKGROUND_JOB_LIST* list : m_shownDialogs ) { - list->Add( job ); + list->CallAfter( + [=]() + { + list->Add( job ); + } ); } } @@ -237,7 +242,7 @@ BACKGROUND_JOB* BACKGROUND_JOBS_MONITOR::Create( const wxString& aName ) } -void BACKGROUND_JOBS_MONITOR::Remove( BACKGROUND_JOB* aJob ) +void BACKGROUND_JOBS_MONITOR::Remove( std::shared_ptr<BACKGROUND_JOB> aJob ) { if( m_shownDialogs.size() > 0 ) { @@ -245,12 +250,17 @@ void BACKGROUND_JOBS_MONITOR::Remove( BACKGROUND_JOB* aJob ) for( BACKGROUND_JOB_LIST* list : m_shownDialogs ) { - list->Remove( aJob ); + list->CallAfter( + [=]() + { + list->Remove( aJob ); + } ); } } + std::lock_guard<std::shared_mutex> lock( m_mutex ); m_jobs.erase( std::remove_if( m_jobs.begin(), m_jobs.end(), - [&]( BACKGROUND_JOB* job ) + [&]( std::shared_ptr<BACKGROUND_JOB> job ) { return job == aJob; } ) ); @@ -259,12 +269,14 @@ void BACKGROUND_JOBS_MONITOR::Remove( BACKGROUND_JOB* aJob ) { for( KISTATUSBAR* statusBar : m_statusBars ) { - statusBar->HideBackgroundProgressBar(); - statusBar->SetBackgroundStatusText( wxT( "" ) ); + statusBar->CallAfter( + [=]() + { + statusBar->HideBackgroundProgressBar(); + statusBar->SetBackgroundStatusText( wxT( "" ) ); + } ); } } - - delete aJob; } @@ -286,11 +298,15 @@ void BACKGROUND_JOBS_MONITOR::ShowList( wxWindow* aParent, wxPoint aPos ) { BACKGROUND_JOB_LIST* list = new BACKGROUND_JOB_LIST( aParent, aPos ); - for( BACKGROUND_JOB* job : m_jobs ) + std::shared_lock<std::shared_mutex> lock( m_mutex, std::try_to_lock ); + + for( std::shared_ptr<BACKGROUND_JOB> job : m_jobs ) { list->Add( job ); } + lock.unlock(); + m_shownDialogs.push_back( list ); list->Bind( wxEVT_CLOSE_WINDOW, &BACKGROUND_JOBS_MONITOR::onListWindowClosed, this ); @@ -303,11 +319,12 @@ void BACKGROUND_JOBS_MONITOR::ShowList( wxWindow* aParent, wxPoint aPos ) } -void BACKGROUND_JOBS_MONITOR::jobUpdated( BACKGROUND_JOB* aJob ) +void BACKGROUND_JOBS_MONITOR::jobUpdated( std::shared_ptr<BACKGROUND_JOB> aJob ) { + std::shared_lock<std::shared_mutex> lock( m_mutex, std::try_to_lock ); + // this method is called from the reporters from potentially other threads // we have to guard ui calls with CallAfter - if( m_jobs.size() > 0 ) { //for now, we go and update the status bar if its the first job in the vector @@ -328,6 +345,7 @@ void BACKGROUND_JOBS_MONITOR::jobUpdated( BACKGROUND_JOB* aJob ) } } + for( BACKGROUND_JOB_LIST* list : m_shownDialogs ) { list->CallAfter( diff --git a/include/background_jobs_monitor.h b/include/background_jobs_monitor.h index 9bb92de106..006a70a364 100644 --- a/include/background_jobs_monitor.h +++ b/include/background_jobs_monitor.h @@ -28,6 +28,7 @@ #include <widgets/progress_reporter_base.h> #include <functional> #include <memory> +#include <shared_mutex> #include <vector> class PROGRESS_REPORTER; @@ -43,7 +44,8 @@ class wxCloseEvent; class BACKGROUND_JOB_REPORTER : public PROGRESS_REPORTER_BASE { public: - BACKGROUND_JOB_REPORTER( BACKGROUND_JOBS_MONITOR* aMonitor, BACKGROUND_JOB* aJob ); + BACKGROUND_JOB_REPORTER( BACKGROUND_JOBS_MONITOR* aMonitor, + std::shared_ptr<BACKGROUND_JOB> aJob ); void SetTitle( const wxString& aTitle ) override { @@ -61,7 +63,7 @@ private: bool updateUI() override; BACKGROUND_JOBS_MONITOR* m_monitor; - BACKGROUND_JOB* m_job; + std::shared_ptr<BACKGROUND_JOB> m_job; wxString m_title; wxString m_report; }; @@ -92,12 +94,12 @@ public: * * @param aName is the displayed title for the event */ - BACKGROUND_JOB* Create( const wxString& aName ); + std::shared_ptr<BACKGROUND_JOB> Create( const wxString& aName ); /** * Removes the given background job from any lists and frees it */ - void Remove( BACKGROUND_JOB* job ); + void Remove( std::shared_ptr<BACKGROUND_JOB> job ); /** * Shows the background job list @@ -123,14 +125,22 @@ private: /** * Handles job status updates, intended to be called by BACKGROUND_JOB_REPORTER only */ - void jobUpdated( BACKGROUND_JOB* aJob ); + void jobUpdated( std::shared_ptr<BACKGROUND_JOB> aJob ); BACKGROUND_JOB_LIST* m_jobListDialog; - std::vector<BACKGROUND_JOB*> m_jobs; + /** + * Holds a reference to all active background jobs + * Access to this vector should be protected by locks since threads may Create or Remove at will + * to register their activity + */ + std::vector<std::shared_ptr<BACKGROUND_JOB>> m_jobs; std::vector<BACKGROUND_JOB_LIST*> m_shownDialogs; std::vector<KISTATUSBAR*> m_statusBars; + + /// Mutex to protect access to the m_jobs vector + mutable std::shared_mutex m_mutex; }; #endif \ No newline at end of file diff --git a/kicad/pcm/pcm.h b/kicad/pcm/pcm.h index 298f3a5db7..e901e5800f 100644 --- a/kicad/pcm/pcm.h +++ b/kicad/pcm/pcm.h @@ -401,7 +401,7 @@ private: std::function<void( int )> m_availableUpdateCallback; std::thread m_updateThread; - BACKGROUND_JOB* m_updateBackgroundJob; + std::shared_ptr<BACKGROUND_JOB> m_updateBackgroundJob; }; #endif // PCM_H_