Tighter thread safety for PCM progress dialog.

This commit is contained in:
Jeff Young 2021-11-10 22:11:52 +00:00
parent 94119b365d
commit 59e5c836fc
3 changed files with 99 additions and 119 deletions

View File

@ -25,7 +25,9 @@
#define GAUGE_RANGE 1000
DIALOG_PCM_PROGRESS::DIALOG_PCM_PROGRESS( wxWindow* parent, bool aShowDownloadSection ) :
DIALOG_PCM_PROGRESS_BASE( parent )
DIALOG_PCM_PROGRESS_BASE( parent ),
PROGRESS_REPORTER_BASE( 1 ),
m_finished( false )
#if wxCHECK_VERSION( 3, 1, 0 )
,
m_appProgressIndicator( parent->GetParent(), GAUGE_RANGE )
@ -34,14 +36,11 @@ DIALOG_PCM_PROGRESS::DIALOG_PCM_PROGRESS( wxWindow* parent, bool aShowDownloadSe
#if wxCHECK_VERSION( 3, 1, 0 )
m_appProgressIndicator.Pulse();
#endif
m_cancelled.store( false );
m_reporter->SetImmediateMode();
m_downloadGauge->SetRange( GAUGE_RANGE );
m_overallGauge->SetRange( GAUGE_RANGE );
SetOverallProgressPhases( 1 );
if( !aShowDownloadSection )
m_panelDownload->Hide();
}
@ -49,45 +48,32 @@ DIALOG_PCM_PROGRESS::DIALOG_PCM_PROGRESS( wxWindow* parent, bool aShowDownloadSe
void DIALOG_PCM_PROGRESS::OnCancelClicked( wxCommandEvent& event )
{
SetNumPhases( 1 );
SetOverallProgress( 1, 1 );
Report( _( "Aborting remaining tasks." ) );
m_cancelled.store( true );
m_buttonCancel->Disable();
m_finished.store( true );
}
void DIALOG_PCM_PROGRESS::OnCloseClicked( wxCommandEvent& event )
{
EndModal( wxID_OK );
m_progress.store( m_maxProgress );
}
void DIALOG_PCM_PROGRESS::Report( const wxString& aText, SEVERITY aSeverity )
{
CallAfter(
[=]
{
m_reporter->Report( aText, aSeverity );
} );
std::lock_guard<std::mutex> guard( m_mutex );
m_reports.push_back( std::make_pair( aText, aSeverity ) );
}
void DIALOG_PCM_PROGRESS::SetDownloadProgress( uint64_t aDownloaded, uint64_t aTotal )
{
if( aDownloaded > aTotal )
aDownloaded = aTotal;
int value = 0;
if( aTotal > 0 )
value = aDownloaded * GAUGE_RANGE / aTotal;
CallAfter(
[=]
{
m_downloadText->SetLabel( wxString::Format( _( "Downloaded %lld/%lld Kb" ),
toKb( aDownloaded ), toKb( aTotal ) ) );
m_downloadGauge->SetValue( value );
} );
m_downloaded.store( std::min( aDownloaded, aTotal ) );
m_downloadTotal.store( aTotal );
}
@ -99,59 +85,69 @@ uint64_t DIALOG_PCM_PROGRESS::toKb( uint64_t aValue )
void DIALOG_PCM_PROGRESS::SetOverallProgress( uint64_t aProgress, uint64_t aTotal )
{
double current = ( m_currentPhase + aProgress / (double) aTotal ) / m_overallPhases;
if( current > 1.0 )
current = 1.0;
int value = current * GAUGE_RANGE;
CallAfter(
[=]
{
m_overallGauge->SetValue( value );
#if wxCHECK_VERSION( 3, 1, 0 )
m_appProgressIndicator.SetValue( value );
#endif
} );
}
void DIALOG_PCM_PROGRESS::SetOverallProgressPhases( int aPhases )
{
m_currentPhase = 0;
m_overallPhases = aPhases;
}
void DIALOG_PCM_PROGRESS::AdvanceOverallProgressPhase()
{
m_currentPhase++;
SetOverallProgress( 0, 1 );
m_overallProgress.store( std::min( aProgress, aTotal ) );
m_overallProgressTotal.store( aTotal );
}
void DIALOG_PCM_PROGRESS::SetFinished()
{
CallAfter(
[this]
{
m_buttonCancel->Disable();
m_buttonClose->Enable();
} );
m_finished.store( true );
}
void DIALOG_PCM_PROGRESS::SetDownloadsFinished()
bool DIALOG_PCM_PROGRESS::updateUI()
{
CallAfter(
[this]
{
m_downloadText->SetLabel( _( "All downloads finished" ) );
} );
int phase = m_phase.load();
int phases = m_numPhases.load();
double current = m_overallProgress.load() / (double) m_overallProgressTotal.load();
if( phases > 0 )
current = ( phase + current ) / phases;
if( current > 1.0 )
current = 1.0;
m_overallGauge->SetValue( current * GAUGE_RANGE );
#if wxCHECK_VERSION( 3, 1, 0 )
m_appProgressIndicator.SetValue( current * GAUGE_RANGE );
#endif
if( m_downloadTotal.load() == 0 )
{
m_downloadText->SetLabel( wxEmptyString );
m_downloadGauge->SetValue( 0 );
}
else
{
m_downloadText->SetLabel( wxString::Format( _( "Downloaded %lld/%lld Kb" ),
toKb( m_downloaded ),
toKb( m_downloadTotal ) ) );
current = m_downloaded.load() / (double) m_downloadTotal.load();
if( current > 1.0 )
current = 1.0;
m_downloadGauge->SetValue( current * GAUGE_RANGE );
}
std::lock_guard<std::mutex> guard( m_mutex );
for( const std::pair<wxString, SEVERITY>& pair : m_reports )
m_reporter->Report( pair.first, pair.second );
m_reports.clear();
if( m_finished.load() )
{
m_buttonCancel->Disable();
m_buttonClose->Enable();
}
wxYield();
return true;
}
bool DIALOG_PCM_PROGRESS::IsCancelled()
{
return m_cancelled.load();
}

View File

@ -27,6 +27,8 @@
#include <atomic>
#if wxCHECK_VERSION( 3, 1, 0 )
#include <wx/appprogress.h>
#include <widgets/progress_reporter_base.h>
#endif
@ -35,11 +37,8 @@
*
* This dialog is designed to work with PCM_TASK_MANAGER's threading system.
* Some of it's methods are safe to call from a non-UI thread.
*
* SetOverallProgress(), SetOverallProgressPhases() and AdvanceOverallProgressPhase()
* are meant to be called from the same thread.
*/
class DIALOG_PCM_PROGRESS : public DIALOG_PCM_PROGRESS_BASE
class DIALOG_PCM_PROGRESS : public DIALOG_PCM_PROGRESS_BASE, public PROGRESS_REPORTER_BASE
{
protected:
// Handlers for DIALOG_PCM_PROGRESS_BASE events.
@ -59,26 +58,25 @@ public:
///< Safe to call from non-UI thread. Sets current overall progress gauge.
void SetOverallProgress( uint64_t aProgress, uint64_t aTotal );
///< Safe to call from non-UI thread. Sets number of phases for overall progress gauge.
void SetOverallProgressPhases( int aPhases );
///< Safe to call from non-UI thread. Increments current phase by one.
void AdvanceOverallProgressPhase();
///< Thread safe. Updates download text.
void SetDownloadsFinished();
///< Thread safe. Disables cancel button, enables close button.
void SetFinished();
///< Thread safe. Return true if cancel was clicked.
bool IsCancelled();
private:
bool updateUI() override;
static uint64_t toKb( uint64_t aValue );
private:
static uint64_t toKb( uint64_t aValue );
int m_currentPhase;
int m_overallPhases;
std::atomic_bool m_cancelled;
std::atomic_int64_t m_downloaded;
std::atomic_int64_t m_downloadTotal;
std::atomic_int64_t m_overallProgress;
std::atomic_int64_t m_overallProgressTotal;
std::atomic_bool m_finished;
std::vector< std::pair<wxString, SEVERITY> > m_reports;
#if wxCHECK_VERSION( 3, 1, 0 )
wxAppProgressIndicator m_appProgressIndicator;
#endif

View File

@ -23,8 +23,6 @@
#include "kicad_curl/kicad_curl_easy.h"
#include "pcm_task_manager.h"
#include "paths.h"
#include "picosha2.h"
#include "reporter.h"
#include "wxstream_helper.h"
@ -33,7 +31,6 @@
#include <unordered_set>
#include <wx/dir.h>
#include <wx/filename.h>
#include <wx/fs_zip.h>
#include <wx/msgdlg.h>
#include <wx/sstream.h>
#include <wx/wfstream.h>
@ -87,14 +84,13 @@ void PCM_TASK_MANAGER::DownloadAndInstall( const PCM_PACKAGE& aPackage, const wx
if( !hash_match )
{
m_reporter->Report(
wxString::Format(
_( "Downloaded archive hash for package %s does not match "
"repository entry. This may indicate a problem with the "
"package, if the issue persists report this to repository "
"maintainers." ),
aPackage.identifier ),
RPT_SEVERITY_ERROR );
m_reporter->Report( wxString::Format( _( "Downloaded archive hash for package "
"%s does not match repository entry. "
"This may indicate a problem with the "
"package, if the issue persists "
"report this to repository maintainers." ),
aPackage.identifier ),
RPT_SEVERITY_ERROR );
}
else
{
@ -136,13 +132,10 @@ int PCM_TASK_MANAGER::downloadFile( const wxString& aFilePath, const wxString& u
TRANSFER_CALLBACK callback = [&]( size_t dltotal, size_t dlnow, size_t ultotal, size_t ulnow )
{
if( dltotal > 1024 )
{
m_reporter->SetDownloadProgress( dlnow, dltotal );
}
else
{
m_reporter->SetDownloadProgress( 0.0, 0.0 );
}
return m_reporter->IsCancelled();
};
@ -340,7 +333,7 @@ void PCM_TASK_MANAGER::InstallFromFile( wxWindow* aParent, const wxString& aFile
m_pcm->MarkInstalled( package, package.versions[0].version, "" );
m_reporter->SetFinished();
m_reporter->ShowModal();
m_reporter->KeepRefreshing( true );
m_reporter->Destroy();
m_reporter.reset();
}
@ -393,9 +386,11 @@ void PCM_TASK_MANAGER::RunQueue( wxWindow* aParent )
{
m_reporter = std::make_unique<DIALOG_PCM_PROGRESS>( aParent );
m_reporter->SetOverallProgressPhases( m_download_queue.size() + m_install_queue.size() );
m_reporter->SetNumPhases( m_download_queue.size() + m_install_queue.size() );
m_reporter->Show();
wxSafeYield();
std::mutex mutex;
std::condition_variable condvar;
bool download_complete = false;
@ -411,8 +406,6 @@ void PCM_TASK_MANAGER::RunQueue( wxWindow* aParent )
condvar.notify_all();
}
m_reporter->SetDownloadsFinished();
std::unique_lock<std::mutex> lock( mutex );
download_complete = true;
condvar.notify_all();
@ -439,7 +432,7 @@ void PCM_TASK_MANAGER::RunQueue( wxWindow* aParent )
PCM_TASK task;
m_install_queue.pop( task );
task();
m_reporter->AdvanceOverallProgressPhase();
m_reporter->AdvancePhase();
}
lock.lock();
@ -447,19 +440,12 @@ void PCM_TASK_MANAGER::RunQueue( wxWindow* aParent )
} while( ( !m_install_queue.empty() || !download_complete )
&& !m_reporter->IsCancelled() );
if( m_reporter->IsCancelled() )
{
m_reporter->SetOverallProgressPhases( 1 );
m_reporter->SetOverallProgress( 1, 1 );
m_reporter->Report( _( "Aborting remaining tasks." ) );
}
m_reporter->Report( _( "Done." ) );
m_reporter->SetFinished();
} );
m_reporter->ShowModal();
m_reporter->KeepRefreshing( true );
m_reporter->Destroy();
m_reporter.reset();