From 26c7cd4f22f82cceff147dbe9a47fdbb2ff06cb3 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Thu, 11 Jun 2020 18:33:44 +0200 Subject: Disconnect signal after extraction to avoid calling them on destroyed widgets. --- src/installationmanager.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index f57872fe..b1eaf1d3 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -163,7 +163,7 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool installationProgress->cancel(); installationProgress->hide(); installationProgress->deleteLater(); - }); + }); installationProgress->setWindowFlags( installationProgress->windowFlags() & (~Qt::WindowContextHelpButtonHint)); if (!title.isEmpty()) { @@ -222,6 +222,11 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool // Wait for future to complete: loop.exec(); + + // There might still be signal in the queue at this point but the progress dialog is + // going to be destroyed so we must disconnect the signals: + disconnect(this, &InstallationManager::progressUpdate, installationProgress, &QProgressDialog::setValue); + disconnect(this, &InstallationManager::progressFileChange, installationProgress, &QProgressDialog::setLabelText); future = futureWatcher.future(); } -- cgit v1.3.1 From 93b1cea0d30e635cad7988dd65578bbbc1abecc7 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Thu, 11 Jun 2020 20:09:00 +0200 Subject: Rollback to using QProcessEvents but use WaitForMoreEvents. --- src/installationmanager.cpp | 50 ++++++++++++++++++++++++--------------------- src/installationmanager.h | 7 +------ 2 files changed, 28 insertions(+), 29 deletions(-) (limited to 'src') diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index b1eaf1d3..8500fec0 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -180,37 +180,34 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool installationProgress->setAutoReset(false); // Connect signals emitted by the extraction callback to the progress dialog slots: - connect(this, &InstallationManager::progressUpdate, installationProgress, &QProgressDialog::setValue); - connect(this, &InstallationManager::progressFileChange, installationProgress, &QProgressDialog::setLabelText); + QEventLoop loop; + connect(this, &InstallationManager::progressUpdate, &loop, &QEventLoop::wakeUp, Qt::QueuedConnection); // Cancelling progress only cancel the extraction, we do not force exiting the event-loop: connect(installationProgress, &QProgressDialog::canceled, [this]() { m_ArchiveHandler->cancel(); }); - installationProgress->show(); + int currentProgress = 0; + QString currentFileName; // The callbacks: - auto progressCallback = [this](auto progressType, uint64_t current, uint64_t total) { + auto progressCallback = [this, ¤tProgress](auto progressType, uint64_t current, uint64_t total) { if (progressType == Archive::ProgressType::EXTRACTION) { - int progress = static_cast(100 * current / total); - emit progressUpdate(progress); + currentProgress = static_cast(100 * current / total); + emit progressUpdate(); } }; - Archive::FileChangeCallback fileChangeCallback = [this](auto changeType, std::wstring const& file) { + Archive::FileChangeCallback fileChangeCallback = [this, ¤tFileName](auto changeType, std::wstring const& file) { if (changeType == Archive::FileChangeType::EXTRACTION_START) { - emit progressFileChange(QString::fromStdWString(file)); + currentFileName = QString::fromStdWString(file); + emit progressUpdate(); } }; - // Future watcher to exit the loop: + // unpack only the files we need for the installer QFutureWatcher futureWatcher; - - QEventLoop loop(this); - connect( - &futureWatcher, &QFutureWatcher::finished, - &loop, &QEventLoop::quit, + connect(&futureWatcher, &QFutureWatcher::finished, + &loop, &QEventLoop::wakeUp, Qt::QueuedConnection); - - // unpack only the files we need for the installer futureWatcher.setFuture(QtConcurrent::run([&]() -> bool { return m_ArchiveHandler->extract( extractPath.toStdWString(), @@ -220,14 +217,21 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool ); })); - // Wait for future to complete: - loop.exec(); + installationProgress->setModal(true); + installationProgress->show(); + + do { + if (currentProgress != installationProgress->value()) { + installationProgress->setValue(currentProgress); + } + if (currentFileName != installationProgress->labelText()) { + installationProgress->setLabelText(currentFileName); + } + loop.processEvents(QEventLoop::AllEvents | QEventLoop::WaitForMoreEvents); + } while (!futureWatcher.isFinished()); + + installationProgress->hide(); - // There might still be signal in the queue at this point but the progress dialog is - // going to be destroyed so we must disconnect the signals: - disconnect(this, &InstallationManager::progressUpdate, installationProgress, &QProgressDialog::setValue); - disconnect(this, &InstallationManager::progressFileChange, installationProgress, &QProgressDialog::setLabelText); - future = futureWatcher.future(); } diff --git a/src/installationmanager.h b/src/installationmanager.h index 3da74ec4..695f2ed6 100644 --- a/src/installationmanager.h +++ b/src/installationmanager.h @@ -223,12 +223,7 @@ signals: /** * @brief Progress update from the extraction. */ - void progressUpdate(int percentage); - - /** - * @brief File change update from the extraction. - */ - void progressFileChange(QString const& value); + void progressUpdate(); private: -- cgit v1.3.1 From a769bc00611ecba57f9abd5832450e19127fb930 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Thu, 11 Jun 2020 20:12:39 +0200 Subject: Add comment explaining the usage of the QEventLoop. --- src/installationmanager.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index 8500fec0..e21a1006 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -179,7 +179,9 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool // extracted... installationProgress->setAutoReset(false); - // Connect signals emitted by the extraction callback to the progress dialog slots: + // Note: Using a loop with a progressUpdate() that only wake-up the loop. The event-loop + // will be used in a loop and not via exec() because connecting to QProgressDialog::setValue + // and using .exec() creates huge recursion that leads to stack-overflow. QEventLoop loop; connect(this, &InstallationManager::progressUpdate, &loop, &QEventLoop::wakeUp, Qt::QueuedConnection); -- cgit v1.3.1 From a64ff3c7bb52007e5bb344eda11026116b4cb273 Mon Sep 17 00:00:00 2001 From: Mikaël Capelle Date: Thu, 11 Jun 2020 20:25:22 +0200 Subject: Add Qt bug report in comment and use while() instead of do { } while();. --- src/installationmanager.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp index e21a1006..b9f35af8 100644 --- a/src/installationmanager.cpp +++ b/src/installationmanager.cpp @@ -182,6 +182,7 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool // Note: Using a loop with a progressUpdate() that only wake-up the loop. The event-loop // will be used in a loop and not via exec() because connecting to QProgressDialog::setValue // and using .exec() creates huge recursion that leads to stack-overflow. + // See https://bugreports.qt.io/browse/QTBUG-10561 QEventLoop loop; connect(this, &InstallationManager::progressUpdate, &loop, &QEventLoop::wakeUp, Qt::QueuedConnection); @@ -222,15 +223,15 @@ bool InstallationManager::extractFiles(QString extractPath, QString title, bool installationProgress->setModal(true); installationProgress->show(); - do { + while (!futureWatcher.isFinished()) { + loop.processEvents(QEventLoop::AllEvents | QEventLoop::WaitForMoreEvents); if (currentProgress != installationProgress->value()) { installationProgress->setValue(currentProgress); } if (currentFileName != installationProgress->labelText()) { installationProgress->setLabelText(currentFileName); } - loop.processEvents(QEventLoop::AllEvents | QEventLoop::WaitForMoreEvents); - } while (!futureWatcher.isFinished()); + } installationProgress->hide(); -- cgit v1.3.1