1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add reentrancy guard for processEvents() in QProgressDialog::setValue()

Browse Source 
Current implementation of QProgressDialog always calls
QCoreApplication::processEvents() when the user calls
QProgressDialog::setValue() if the PD is modal. For most cases this is
fine, but when using a Qt::WindowModal PD with setValue() connected to
a signal in another thread using Qt::QueuedConnection a reentrancy
issue is present if setValue() is triggered too frequently as the
execution of its previous call may not have finished. If this happens
too many times in a row a stack overflow will occur.

Current documentation notes this potential issue but offers no way it
avoid it while still using QProgressDialog (user must implement a
custom dialog) without resorting to using Qt::BlockingQueuedConnection,
which unnecessarily reduces performance.

Introduces the boolean reentrancy guard "processingEvents" that is
checked before calling QCoreApplication::processEvents() in a modal
PD when setValue() is used. It is set before the first call to
processEvents() and cleared after that call returns. This ensures that
only one invocation of processEvents() is possible from within
setValue() at a time, and thereby minimizes iterations of the main event
loop and eliminates the aforementioned stack overflow condition.

See - https://forum.qt.io/topic/118292/

Fixes: QTBUG-10561
Pick-to: 5.15
Change-Id: Ifa9b91cbb66881981356954ead0906bdc91fab60
Reviewed-by: Samuel Gaist <samuel.gaist@idiap.ch>
This commit is contained in:
Christian Heimlich 2020-09-19 05:53:26 -04:00
parent 169f332eab
commit 10bc093d4f
2 changed files with 30 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/widgets/dialogs/qprogressdialog.cpp
View File

@ -51,6 +51,7 @@
#include "qpushbutton.h"
#include "qtimer.h"
#include "qelapsedtimer.h"
#include "qscopedvaluerollback.h"
#include <private/qdialog_p.h>
#include <limits.h>
@ -71,6 +72,7 @@ public:
shown_once(false),
cancellation_flag(false),
setValue_called(false),
processingEvents(false),
showTime(defaultShowTime),
#ifndef QT_NO_SHORTCUT
escapeShortcut(nullptr),
@ -94,6 +96,7 @@ public:
bool shown_once;
bool cancellation_flag;
bool setValue_called;
bool processingEvents;
QElapsedTimer starttime;
int showTime;
bool autoClose;
@ -660,8 +663,10 @@ void QProgressDialog::setValue(int progress)
d->bar->setValue(progress);
if (d->shown_once) {
if (isModal())
if (isModal() && !d->processingEvents) {
const QScopedValueRollback guard(d->processingEvents, true);
QCoreApplication::processEvents();
}
} else {
if ((!d->setValue_called && progress == 0 /* for compat with Qt < 5.4 */) || progress == minimum()) {
d->starttime.start();

24
tests/auto/widgets/dialogs/qprogressdialog/tst_qprogressdialog.cpp
View File

@ -53,6 +53,7 @@ private Q_SLOTS:
void QTBUG_31046();
void settingCustomWidgets();
void i18n();
void setValueReentrancyGuard();
};
void tst_QProgressDialog::cleanup()
@ -291,5 +292,28 @@ void tst_QProgressDialog::i18n()
QVERIFY(!btn->text().startsWith(xxx));
}
void tst_QProgressDialog::setValueReentrancyGuard()
{
// Tests setValue() of window modal QProgressBar with
// Qt::QueuedConnection:
// This test crashes with a stack overflow if the boolean
// guard "processingEvents" that prevents reentranct calls
// to QCoreApplication::processEvents() within setValue()
// has not been implemented
constexpr int steps = 100; // Should be at least 50 to test for crash
QProgressDialog dlg("Testing setValue reentrancy guard...", QString(), 0, steps);
dlg.setWindowModality(Qt::WindowModal);
dlg.setMinimumDuration(0);
dlg.setAutoReset(false);
// Simulate a quick work loop
for (int i = 0; i <= steps; ++i)
QMetaObject::invokeMethod(&dlg, "setValue", Qt::QueuedConnection, Q_ARG(int, i));
QTRY_COMPARE(dlg.value(), steps);
}
QTEST_MAIN(tst_QProgressDialog)
#include "tst_qprogressdialog.moc"