Review Request 113877: Use QMutableMapIterator for removing KStartupInfoIds

Fri Nov 15 09:48:37 UTC 2013

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113877/
-----------------------------------------------------------

Review request for KDE Frameworks.

Repository: kdelibs

Description
-------

If there were multiple KStartupInfoIds which were removed at the same time, it was possible that the map got changed while being iterated, thus the data corrupted and we hit a crash later on. This changes to a QMutableMapIterator for checking the cleanup and allowing to safely remove ids during the iteration.

Diffs
-----

  tier1/kwindowsystem/autotests/kstartupinfo_unittest.cpp 17890ff 
  tier1/kwindowsystem/src/kstartupinfo.cpp 402cc97 

Diff: http://git.reviewboard.kde.org/r/113877/diff/

Testing
-------

When running the unit tests on the old code base it isn't guaranteed that it crashes as it's a race and also depends on X. Just run a few times and it will certainly hit the abort condition.

For reference the backtrace of the crash before fix:
#0  0x00007ffff50131e5 in __GI_raise (sig=sig at entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff5016398 in __GI_abort () at abort.c:90
#2  0x00007ffff5c3d402 in qt_message_fatal (context=..., message=...) at global/qlogging.cpp:979
#3  0x00007ffff5c3afce in QMessageLogger::fatal (this=0x7fffffffbbc0, msg=0x7ffff5f8d9f8 "ASSERT: \"%s\" in file %s, line %d") at global/qlogging.cpp:384
#4  0x00007ffff5c369e6 in qt_assert (assertion=0x7ffff5f8b7f0 "size == 0 || offset < 0 || size_t(offset) >= sizeof(QArrayData)", file=0x7ffff5f8b7b0 "../../include/QtCore/../../src/corelib/tools/qarraydata.h", line=62)
    at global/qglobal.cpp:2088
#5  0x00007ffff5c29bf8 in QArrayData::data (this=0x6cb830) at ../../include/QtCore/../../src/corelib/tools/qarraydata.h:61
#6  0x00007ffff5c3015e in QTypedArrayData<char>::data (this=0x6cb830) at ../../include/QtCore/../../src/corelib/tools/qarraydata.h:207
#7  0x00007ffff5c2fde7 in QByteArray::constData (this=0x6f4760) at ../../include/QtCore/../../src/corelib/tools/qbytearray.h:433
#8  0x00007ffff5c53333 in qstrcmp (str1=..., str2=...) at tools/qbytearray.cpp:347
#9  0x00007ffff7ba4625 in operator< (a1=..., a2=...) at /opt/qt5/include/QtCore/qbytearray.h:550
#10 0x00007ffff7ba0eb9 in KStartupInfoId::operator< (this=0x693858, id_P=...) at /home/martin/src/kf5/kdelibs-frameworks/tier1/kwindowsystem/src/kstartupinfo.cpp:1175
#11 0x00007ffff7ba67b5 in qMapLessThanKey<KStartupInfoId> (key1=..., key2=...) at /opt/qt5/include/QtCore/qmap.h:75
#12 0x00007ffff7ba6ef3 in QMapNode<KStartupInfoId, KStartupInfo::Data>::lowerBound (this=0x693840, akey=...) at /opt/qt5/include/QtCore/qmap.h:145
#13 0x00007ffff7ba66b4 in QMapData<KStartupInfoId, KStartupInfo::Data>::findNode (this=0x700870, akey=...) at /opt/qt5/include/QtCore/qmap.h:292
#14 0x00007ffff7ba5a73 in QMap<KStartupInfoId, KStartupInfo::Data>::remove (this=0x65acc0, akey=...) at /opt/qt5/include/QtCore/qmap.h:897
#15 0x00007ffff7b9dd42 in KStartupInfo::Private::remove_startup_info_internal (this=0x65acb0, id_P=...) at /home/martin/src/kf5/kdelibs-frameworks/tier1/kwindowsystem/src/kstartupinfo.cpp:399
#16 0x00007ffff7ba039b in KStartupInfo::Private::startups_cleanup_internal (this=0x65acb0, age_P=true) at /home/martin/src/kf5/kdelibs-frameworks/tier1/kwindowsystem/src/kstartupinfo.cpp:984
#17 0x00007ffff7ba02bf in KStartupInfo::Private::startups_cleanup (this=0x65acb0) at /home/martin/src/kf5/kdelibs-frameworks/tier1/kwindowsystem/src/kstartupinfo.cpp:965
#18 0x00007ffff7ba3d1c in KStartupInfo::qt_static_metacall (_o=0x7fffffffdc60, _c=QMetaObject::InvokeMetaMethod, _id=3, _a=0x7fffffffbf70) at /opt/build/kf5/kdelibs-frameworks/tier1/kwindowsystem/src/moc_kstartupinfo.cpp:106
#19 0x00007ffff5ef8c9f in QMetaObject::activate (sender=0x684d50, signalOffset=3, local_signal_index=0, argv=0x0) at kernel/qobject.cpp:3530
#20 0x00007ffff5ef84f0 in QMetaObject::activate (sender=0x684d50, m=0x7ffff63ab980 <QTimer::staticMetaObject>, local_signal_index=0, argv=0x0) at kernel/qobject.cpp:3405
#21 0x00007ffff5f88fc3 in QTimer::timeout (this=0x684d50) at .moc/moc_qtimer.cpp:189
#22 0x00007ffff5f03bc5 in QTimer::timerEvent (this=0x684d50, e=0x7fffffffca30) at kernel/qtimer.cpp:255
#23 0x00007ffff5ef2314 in QObject::event (this=0x684d50, e=0x7fffffffca30) at kernel/qobject.cpp:1101
#24 0x00007ffff710e24e in QApplicationPrivate::notify_helper (this=0x60f010, receiver=0x684d50, e=0x7fffffffca30) at kernel/qapplication.cpp:3467
#25 0x00007ffff710b805 in QApplication::notify (this=0x7fffffffdc90, receiver=0x684d50, e=0x7fffffffca30) at kernel/qapplication.cpp:2888
#26 0x00007ffff5eb6908 in QCoreApplication::notifyInternal (this=0x7fffffffdc90, receiver=0x684d50, event=0x7fffffffca30) at kernel/qcoreapplication.cpp:878
#27 0x00007ffff5eba4f1 in QCoreApplication::sendEvent (receiver=0x684d50, event=0x7fffffffca30) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:232
#28 0x00007ffff5f2c450 in QTimerInfoList::activateTimers (this=0x654ec0) at kernel/qtimerinfo_unix.cpp:643
#29 0x00007ffff5f2d764 in timerSourceDispatch (source=0x654e60) at kernel/qeventdispatcher_glib.cpp:185
#30 0x00007ffff25a8ea6 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#31 0x00007ffff25a91f8 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#32 0x00007ffff25a929c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#33 0x00007ffff5f2e0f9 in QEventDispatcherGlib::processEvents (this=0x64ed40, flags=...) at kernel/qeventdispatcher_glib.cpp:426
#34 0x00007fffefcca744 in QPAEventDispatcherGlib::processEvents (this=0x64ed40, flags=...) at eventdispatchers/qeventdispatcher_glib.cpp:123
#35 0x00007ffff5eb6e46 in QCoreApplication::processEvents (flags=..., maxtime=11000) at kernel/qcoreapplication.cpp:1076
#36 0x0000000000403ecc in QTest::qWait (ms=11000) at /opt/qt5/include/QtTest/qtestsystem.h:66
#37 0x000000000040690c in KStartupInfo_UnitTest::dontCrashCleanup (this=0x7fffffffdc50) at /home/martin/src/kf5/kdelibs-frameworks/tier1/kwindowsystem/autotests/kstartupinfo_unittest.cpp:128

Thanks,

Martin Gräßlin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131115/10034ead/attachment-0001.html>