Make sure that signal disconnects don't disconnect too much

There has been a latent bug forever in QtDBus that would make a signal
disconnect actually disconnect too much. The reason is that
disconnectNotify() is called every time a signal is disconnected from
a receiver, but that doesn't mean it was the last connection.

This test checks whether disconnecting from voidSignal() to our test
receiver will also disconnect from exitLoop(). If it does, we'll get a
timeout. I could have implemented it with two receivers, but in the
buggy case, it would always fail first in the timeout verification.

Change-Id: I5766d8a38594eb25e65b304913251303660fad41
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@digia.com>
bb10
Thiago Macieira 2013-03-13 11:56:08 -07:00 committed by The Qt Project
parent d1b4857d17
commit d2a98df9bc
2 changed files with 91 additions and 1 deletions

View File

@ -609,7 +609,7 @@ void QDBusAbstractInterface::disconnectNotify(const QMetaMethod &signal)
return;
QDBusConnectionPrivate *conn = d->connectionPrivate();
if (conn)
if (conn && !isSignalConnected(signal))
conn->disconnectRelay(d->service, d->path, d->interface,
this, signal);
}

View File

@ -159,6 +159,11 @@ private slots:
void followSignal();
void connectDisconnect_data();
void connectDisconnect();
void connectDisconnectPeer_data();
void connectDisconnectPeer();
void createErrors_data();
void createErrors();
@ -196,6 +201,15 @@ private:
QProcess proc;
};
class SignalReceiver : public QObject
{
Q_OBJECT
public:
int callCount;
SignalReceiver() : callCount(0) {}
public slots:
void receive() { ++callCount; }
};
tst_QDBusAbstractInterface::tst_QDBusAbstractInterface()
{
@ -1022,6 +1036,82 @@ void tst_QDBusAbstractInterface::followSignal()
con.interface()->unregisterService(serviceToFollow);
}
void tst_QDBusAbstractInterface::connectDisconnect_data()
{
QTest::addColumn<int>("connectCount");
QTest::addColumn<int>("disconnectCount");
// we don't actually need multiple disconnects
// QObject::disconnect() disconnects all matching rules
// we'd have to use QMetaObject::disconnectOne if we wanted just one
QTest::newRow("null") << 0 << 0;
QTest::newRow("connect-disconnect") << 1 << 1;
QTest::newRow("connect-twice") << 2 << 0;
QTest::newRow("connect-twice-disconnect") << 2 << 1;
}
void tst_QDBusAbstractInterface::connectDisconnect()
{
QFETCH(int, connectCount);
QFETCH(int, disconnectCount);
Pinger p = getPinger();
QVERIFY2(p, "Not connected to D-Bus");
// connect the exitLoop slot first
// if the disconnect() below does something weird, we'll get a timeout
QTestEventLoop::instance().connect(p.data(), SIGNAL(voidSignal()), SLOT(exitLoop()));
SignalReceiver sr;
for (int i = 0; i < connectCount; ++i)
sr.connect(p.data(), SIGNAL(voidSignal()), SLOT(receive()));
if (disconnectCount)
QObject::disconnect(p.data(), SIGNAL(voidSignal()), &sr, SLOT(receive()));
emit targetObj.voidSignal();
QTestEventLoop::instance().enterLoop(2);
QVERIFY(!QTestEventLoop::instance().timeout());
if (disconnectCount != 0)
QCOMPARE(sr.callCount, 0);
else
QCOMPARE(sr.callCount, connectCount);
}
void tst_QDBusAbstractInterface::connectDisconnectPeer_data()
{
connectDisconnect_data();
}
void tst_QDBusAbstractInterface::connectDisconnectPeer()
{
QFETCH(int, connectCount);
QFETCH(int, disconnectCount);
Pinger p = getPingerPeer();
QVERIFY2(p, "Not connected to D-Bus");
// connect the exitLoop slot first
// if the disconnect() below does something weird, we'll get a timeout
QTestEventLoop::instance().connect(p.data(), SIGNAL(voidSignal()), SLOT(exitLoop()));
SignalReceiver sr;
for (int i = 0; i < connectCount; ++i)
sr.connect(p.data(), SIGNAL(voidSignal()), SLOT(receive()));
if (disconnectCount)
QObject::disconnect(p.data(), SIGNAL(voidSignal()), &sr, SLOT(receive()));
QDBusMessage req = QDBusMessage::createMethodCall(serviceName, objectPath, interfaceName, "voidSignal");
QVERIFY(QDBusConnection::sessionBus().send(req));
QTestEventLoop::instance().enterLoop(2);
QVERIFY(!QTestEventLoop::instance().timeout());
if (disconnectCount != 0)
QCOMPARE(sr.callCount, 0);
else
QCOMPARE(sr.callCount, connectCount);
}
void tst_QDBusAbstractInterface::createErrors_data()
{
QTest::addColumn<QString>("service");