Fix race condition in ~QObject

while using QOrderedMutexLocker::relock we might unlock our mutex
protecting the 'senders' list for a short moment. Another thread may then
modify or remove element on the list.
Therefore, we need to recheck the consistency of the list once we did
that.

Also, we cannot call removeSender because that will remove every
connections, making impossible for another object destroyed in the same
time to clean up the senders list, so call derefSender instead.

Reviewed-by: Brad

2 files changed, 11 insertions, 20 deletions

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 05015c0..f1a1eb5 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -345,18 +345,6 @@ void QObjectPrivate::derefSender(QObject *sender, int signal)
     // Q_ASSERT_X(false, "QObjectPrivate::derefSender", "sender not found");
 }
 
-void QObjectPrivate::removeSender(QObject *sender, int signal)
-{
-    for (int i = 0; i < senders.count(); ++i) {
-        Sender &s = senders[i];
-        if (s.sender == sender && s.signal == signal) {
-            senders.removeAt(i);
-            return;
-        }
-    }
-    // Q_ASSERT_X(false, "QObjectPrivate::removeSender", "sender not found");
-}
-
 QObjectPrivate::Sender *QObjectPrivate::setCurrentSender(QObject *receiver,
                                                          Sender *sender)
 {
@@ -790,7 +778,7 @@ QObject::~QObject()
                     bool needToUnlock = QOrderedMutexLocker::relock(locker.mutex(), m);
                     c = &connectionList[i];
                     if (c->receiver)
-                        c->receiver->d_func()->removeSender(this, signal);
+                        c->receiver->d_func()->derefSender(this, signal);
                     if (needToUnlock)
                         m->unlock();
 
@@ -811,18 +799,22 @@ QObject::~QObject()
         }
 
         // disconnect all senders
-        for (int i = 0; i < d->senders.count(); ++i) {
+        for (int i = 0; i < d->senders.count(); ) {
             QObjectPrivate::Sender *s = &d->senders[i];
-            if (!s->sender)
-                continue;
 
             QMutex *m = &s->sender->d_func()->threadData->mutex;
             bool needToUnlock = QOrderedMutexLocker::relock(locker.mutex(), m);
-            s = &d->senders[i];
-            if (s->sender)
-                s->sender->d_func()->removeReceiver(s->signal, this);
+            if (m < locker.mutex()) {
+                if (i >= d->senders.count() || s != &d->senders[i]) {
+                    if (needToUnlock)
+                        m->unlock();
+                    continue;
+                }
+            }
+            s->sender->d_func()->removeReceiver(s->signal, this);
             if (needToUnlock)
                 m->unlock();
+            ++i;
         }
 
         d->senders.clear();
diff --git a/src/corelib/kernel/qobject_p.h b/src/corelib/kernel/qobject_p.h
index b324334..0eed938 100644
--- a/src/corelib/kernel/qobject_p.h
+++ b/src/corelib/kernel/qobject_p.h
@@ -163,7 +163,6 @@ public:
     QList<Sender> senders;
     void refSender(QObject *sender, int signal);
     void derefSender(QObject *sender, int signal);
-    void removeSender(QObject *sender, int signal);
 
     static Sender *setCurrentSender(QObject *receiver,
                                     Sender *sender);