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

QDialogButtonBox: prevent crashes on deletions in slots

Browse Source 
As usual, user code connected to signals emitted from Qt may destroy
an object's internal status while a signal is being emitted.

Guard a bit QDialogButtonBox signal emissions to prevent
crashes in case the button or a dialog get deleted from a slot
connected to clicked(). Also, be sure to emit the corresponding
accepted/rejected/etc. signal.

Change-Id: I7b1888070a8f2f56aa60923a17f90fb5efef145c
Task-number: QTBUG-45835
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@theqtcompany.com>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
This commit is contained in:
Giuseppe D'Angelo 2015-05-08 13:49:54 +02:00
parent 4b7d70886f
commit 1ac1ae05f5
2 changed files with 77 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/widgets/widgets/qdialogbuttonbox.cpp
View File

@ -852,9 +852,19 @@ void QDialogButtonBoxPrivate::_q_handleButtonClicked()
{
Q_Q(QDialogButtonBox);
if (QAbstractButton *button = qobject_cast<QAbstractButton *>(q->sender())) {
// Can't fetch this *after* emitting clicked, as clicked may destroy the button
// or change its role. Now changing the role is not possible yet, but arguably
// both clicked and accepted/rejected/etc. should be emitted "atomically"
// depending on whatever role the button had at the time of the click.
const QDialogButtonBox::ButtonRole buttonRole = q->buttonRole(button);
QPointer<QDialogButtonBox> guard(q);
emit q->clicked(button);
switch (q->buttonRole(button)) {
if (!guard)
return;
switch (buttonRole) {
case QPlatformDialogHelper::AcceptRole:
case QPlatformDialogHelper::YesRole:
emit q->accepted();

66
tests/auto/widgets/widgets/qdialogbuttonbox/tst_qdialogbuttonbox.cpp
View File

@ -90,6 +90,7 @@ private slots:
// void buttons();
void testDelete();
void testSignalEmissionAfterDelete_QTBUG_45835();
void testRemove();
void testMultipleAdd();
void testStandardButtonMapping_data();
@ -111,6 +112,7 @@ private:
tst_QDialogButtonBox::tst_QDialogButtonBox()
{
qRegisterMetaType<QAbstractButton *>();
}
tst_QDialogButtonBox::~tst_QDialogButtonBox()
@ -414,6 +416,70 @@ void tst_QDialogButtonBox::testDelete()
QCOMPARE(buttonBox.buttons().count(), 0);
}
class ObjectDeleter : public QObject
{
Q_OBJECT
public slots:
void deleteButton(QAbstractButton *button)
{
delete button;
}
void deleteSender()
{
delete sender();
}
};
void tst_QDialogButtonBox::testSignalEmissionAfterDelete_QTBUG_45835()
{
{
QDialogButtonBox buttonBox;
QCOMPARE(buttonBox.buttons().count(), 0);
QSignalSpy buttonClickedSpy(&buttonBox, &QDialogButtonBox::clicked);
QVERIFY(buttonClickedSpy.isValid());
QSignalSpy buttonBoxAcceptedSpy(&buttonBox, &QDialogButtonBox::accepted);
QVERIFY(buttonBoxAcceptedSpy.isValid());
QPushButton *button = buttonBox.addButton("Test", QDialogButtonBox::AcceptRole);
QCOMPARE(buttonBox.buttons().count(), 1);
ObjectDeleter objectDeleter;
connect(&buttonBox, &QDialogButtonBox::clicked, &objectDeleter, &ObjectDeleter::deleteButton);
button->click();
QCOMPARE(buttonBox.buttons().count(), 0);
QCOMPARE(buttonClickedSpy.count(), 1);
QCOMPARE(buttonBoxAcceptedSpy.count(), 1);
}
{
QPointer<QDialogButtonBox> buttonBox(new QDialogButtonBox);
QCOMPARE(buttonBox->buttons().count(), 0);
QSignalSpy buttonClickedSpy(buttonBox.data(), &QDialogButtonBox::clicked);
QVERIFY(buttonClickedSpy.isValid());
QSignalSpy buttonBoxAcceptedSpy(buttonBox.data(), &QDialogButtonBox::accepted);
QVERIFY(buttonBoxAcceptedSpy.isValid());
QPushButton *button = buttonBox->addButton("Test", QDialogButtonBox::AcceptRole);
QCOMPARE(buttonBox->buttons().count(), 1);
ObjectDeleter objectDeleter;
connect(buttonBox.data(), &QDialogButtonBox::clicked, &objectDeleter, &ObjectDeleter::deleteSender);
button->click();
QVERIFY(buttonBox.isNull());
QCOMPARE(buttonClickedSpy.count(), 1);
QCOMPARE(buttonBoxAcceptedSpy.count(), 0);
}
}
void tst_QDialogButtonBox::testMultipleAdd()
{
// Add a button into the thing multiple times.