From f805020410c8ccb0cd223988565bcabde1c5806b Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Fri, 15 Nov 2013 09:46:02 +0100 Subject: Fix a race that occurred as we unlock the mutex to destroy the functor in ~QObject When we unlock the mutex, we need to take in account that the Connection pointed by 'node' may be destroyed in another thread while it is unlocked Doing 'node->prev = &node' will make sure that 'node' is actually updated when it is destroyed. Setting isSlotObject under the mutex is safer and ensure that no other thread will attempt to deref the object. The regression was introduced in 5885b8f775998c30d53f40b7f368c5f6364e6df4 tst_qobjectrace was updated to catch races arising when we are connecting with function pointers. Change-Id: Ia0d11ae8df563dad97eb86993a786b579b28cd03 Reviewed-by: Allan Sandfeld Jensen Reviewed-by: Lars Knoll --- tests/auto/other/qobjectrace/tst_qobjectrace.cpp | 76 ++++++++++++++++++------ 1 file changed, 57 insertions(+), 19 deletions(-) (limited to 'tests/auto') diff --git a/tests/auto/other/qobjectrace/tst_qobjectrace.cpp b/tests/auto/other/qobjectrace/tst_qobjectrace.cpp index ab05c64fe5..71a90e83f7 100644 --- a/tests/auto/other/qobjectrace/tst_qobjectrace.cpp +++ b/tests/auto/other/qobjectrace/tst_qobjectrace.cpp @@ -47,6 +47,12 @@ enum { OneMinute = 60 * 1000, TwoMinutes = OneMinute * 2 }; + +struct Functor +{ + void operator()() const {}; +}; + class tst_QObjectRace: public QObject { Q_OBJECT @@ -122,11 +128,7 @@ signals: private slots: void checkStopWatch() { -#if defined(Q_OS_WINCE) || defined(Q_OS_VXWORKS) - if (stopWatch.elapsed() >= OneMinute / 2) -#else - if (stopWatch.elapsed() >= OneMinute) -#endif + if (stopWatch.elapsed() >= 5000) quit(); QObject o; @@ -188,16 +190,34 @@ class MyObject : public QObject void signal7(); }; +namespace { +const char *_slots[] = { SLOT(slot1()) , SLOT(slot2()) , SLOT(slot3()), + SLOT(slot4()) , SLOT(slot5()) , SLOT(slot6()), + SLOT(slot7()) }; + +const char *_signals[] = { SIGNAL(signal1()), SIGNAL(signal2()), SIGNAL(signal3()), + SIGNAL(signal4()), SIGNAL(signal5()), SIGNAL(signal6()), + SIGNAL(signal7()) }; +typedef void (MyObject::*PMFType)(); +const PMFType _slotsPMF[] = { &MyObject::slot1, &MyObject::slot2, &MyObject::slot3, + &MyObject::slot4, &MyObject::slot5, &MyObject::slot6, + &MyObject::slot7 }; + +const PMFType _signalsPMF[] = { &MyObject::signal1, &MyObject::signal2, &MyObject::signal3, + &MyObject::signal4, &MyObject::signal5, &MyObject::signal6, + &MyObject::signal7 }; + +} class DestroyThread : public QThread { Q_OBJECT - QObject **objects; + MyObject **objects; int number; public: - void setObjects(QObject **o, int n) + void setObjects(MyObject **o, int n) { objects = o; number = n; @@ -206,8 +226,29 @@ public: } void run() { - for(int i = 0; i < number; i++) + for (int i = number-1; i >= 0; --i) { + /* Do some more connection and disconnection between object in this thread that have not been destroyed yet */ + + const int nAlive = i+1; + connect (objects[((i+1)*31) % nAlive], _signals[(12*i)%7], objects[((i+2)*37) % nAlive], _slots[(15*i+2)%7] ); + disconnect(objects[((i+1)*31) % nAlive], _signals[(12*i)%7], objects[((i+2)*37) % nAlive], _slots[(15*i+2)%7] ); + + connect (objects[((i+4)*41) % nAlive], _signalsPMF[(18*i)%7], objects[((i+5)*43) % nAlive], _slotsPMF[(19*i+2)%7] ); + disconnect(objects[((i+4)*41) % nAlive], _signalsPMF[(18*i)%7], objects[((i+5)*43) % nAlive], _slotsPMF[(19*i+2)%7] ); + + QMetaObject::Connection c = connect(objects[((i+5)*43) % nAlive], _signalsPMF[(9*i+1)%7], Functor()); + disconnect(c); + + disconnect(objects[i], _signalsPMF[(10*i+5)%7], 0, 0); + disconnect(objects[i], _signals[(11*i+6)%7], 0, 0); + + disconnect(objects[i], 0, objects[(i*17+6) % nAlive], 0); + if (i%4 == 1) { + disconnect(objects[i], 0, 0, 0); + } + delete objects[i]; + } } }; @@ -216,27 +257,24 @@ public: void tst_QObjectRace::destroyRace() { - enum { ThreadCount = 10, ObjectCountPerThread = 733, + enum { ThreadCount = 10, ObjectCountPerThread = 2777, ObjectCount = ThreadCount * ObjectCountPerThread }; - const char *_slots[] = { SLOT(slot1()) , SLOT(slot2()) , SLOT(slot3()), - SLOT(slot4()) , SLOT(slot5()) , SLOT(slot6()), - SLOT(slot7()) }; - - const char *_signals[] = { SIGNAL(signal1()), SIGNAL(signal2()), SIGNAL(signal3()), - SIGNAL(signal4()), SIGNAL(signal5()), SIGNAL(signal6()), - SIGNAL(signal7()) }; - - QObject *objects[ObjectCount]; + MyObject *objects[ObjectCount]; for (int i = 0; i < ObjectCount; ++i) objects[i] = new MyObject; - for (int i = 0; i < ObjectCount * 11; ++i) { + for (int i = 0; i < ObjectCount * 17; ++i) { connect(objects[(i*13) % ObjectCount], _signals[(2*i)%7], objects[((i+2)*17) % ObjectCount], _slots[(3*i+2)%7] ); connect(objects[((i+6)*23) % ObjectCount], _signals[(5*i+4)%7], objects[((i+8)*41) % ObjectCount], _slots[(i+6)%7] ); + + connect(objects[(i*67) % ObjectCount], _signalsPMF[(2*i)%7], + objects[((i+1)*71) % ObjectCount], _slotsPMF[(3*i+2)%7] ); + connect(objects[((i+3)*73) % ObjectCount], _signalsPMF[(5*i+4)%7], + objects[((i+5)*79) % ObjectCount], Functor() ); } DestroyThread *threads[ThreadCount]; -- cgit v1.2.3