D20024: Fixes crash when hiding devices
David Hallas
noreply at phabricator.kde.org
Sat Mar 30 06:32:36 GMT 2019
hallas added a comment.
Hi @elvisangelaccio , I have been looking into writing a unit test for the crash, but it turns out we already have one: `PlacesModelItemTest::testHideItem`. But, the test only triggers the crash if it is run before the `testPlaceItem` test case. I have been digging in the code to figure out why the test cases are coupled in this way, but I haven't figured it out, so could you take a look? Here is a diff that triggers the crash:
diff --git a/src/tests/placesitemmodeltest.cpp b/src/tests/placesitemmodeltest.cpp
index fac0931a6..05aa02064 100644
--- a/src/tests/placesitemmodeltest.cpp
+++ b/src/tests/placesitemmodeltest.cpp
@@ -63,13 +63,13 @@ private slots:
void testModelSort();
void testGroups();
void testDeletePlace();
+ void testHideItem();
void testPlaceItem_data();
void testPlaceItem();
void testTearDownDevice();
void testDefaultViewProperties_data();
void testDefaultViewProperties();
void testClear();
- void testHideItem();
void testSystemItems();
void testEditBookmark();
void testEditAfterCreation();
So if I revert the crash fix and run the test like this `valgrind bin/placesitemmodeltest` I see the crash:
==6910== Invalid read of size 8
==6910== at 0x5166A43: KStandardItem::setDataValue(QByteArray const&, QVariant const&) (kstandarditem.cpp:118)
==6910== by 0x178D2B: PlacesItem::setHidden(bool) (placesitem.cpp:96)
==6910== by 0x1575C0: PlacesItemModelTest::testHideItem() (placesitemmodeltest.cpp:520)
==6910== by 0x16AFD3: PlacesItemModelTest::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (placesitemmodeltest.moc:159)
==6910== by 0xC64C900: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (in /usr/lib64/libQt5Core.so.5.11.3)
==6910== by 0x4E5236B: ??? (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E530DC: ??? (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E53650: ??? (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E53B0A: QTest::qRun() (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E53E2A: QTest::qExec(QObject*, int, char**) (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x16AEC3: main (placesitemmodeltest.cpp:947)
==6910== Address 0x2319abd8 is 24 bytes inside a block of size 136 free'd
==6910== at 0x4C3083B: operator delete(void*) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6910== by 0x1787ED: PlacesItem::~PlacesItem() (placesitem.cpp:51)
==6910== by 0x5173C74: KStandardItemModel::removeItem(int) (kstandarditemmodel.cpp:115)
==6910== by 0x17FF7C: PlacesItemModel::onSourceModelDataChanged(QModelIndex const&, QModelIndex const&, QVector<int> const&) (placesitemmodel.cpp:569)
==6910== by 0x1862A5: QtPrivate::FunctorCall<QtPrivate::IndexesList<0, 1, 2>, QtPrivate::List<QModelIndex const&, QModelIndex const&, QVector<int> const&>, void, void (PlacesItemModel::*)(QModelIndex const&, QModelIndex const&, QVector<int> const&)>::call(void (PlacesItemModel::*)(QModelIndex const&, QModelIndex const&, QVector<int> const&), PlacesItemModel*, void**) (qobjectdefs_impl.h:134)
==6910== by 0x185A61: void QtPrivate::FunctionPointer<void (PlacesItemModel::*)(QModelIndex const&, QModelIndex const&, QVector<int> const&)>::call<QtPrivate::List<QModelIndex const&, QModelIndex const&, QVector<int> const&>, void>(void (PlacesItemModel::*)(QModelIndex const&, QModelIndex const&, QVector<int> const&), PlacesItemModel*, void**) (qobjectdefs_impl.h:167)
==6910== by 0x1843C0: QtPrivate::QSlotObject<void (PlacesItemModel::*)(QModelIndex const&, QModelIndex const&, QVector<int> const&), QtPrivate::List<QModelIndex const&, QModelIndex const&, QVector<int> const&>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:396)
==6910== by 0xC66596E: QMetaObject::activate(QObject*, int, int, void**) (in /usr/lib64/libQt5Core.so.5.11.3)
==6910== by 0xC5FB45B: QAbstractItemModel::dataChanged(QModelIndex const&, QModelIndex const&, QVector<int> const&) (in /usr/lib64/libQt5Core.so.5.11.3)
==6910== by 0x56BBC63: KFilePlacesModel::setPlaceHidden(QModelIndex const&, bool) (in /usr/lib64/libKF5KIOFileWidgets.so.5.56.0)
==6910== by 0x17D77B: PlacesItemModel::onItemChanged(int, QSet<QByteArray> const&) (placesitemmodel.cpp:189)
==6910== by 0x5166A3E: KStandardItem::setDataValue(QByteArray const&, QVariant const&) (kstandarditem.cpp:117)
==6910== Block was alloc'd at
==6910== at 0x4C2F77F: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==6910== by 0x17EB09: PlacesItemModel::addItemFromSourceModel(QModelIndex const&) (placesitemmodel.cpp:392)
==6910== by 0x18088F: PlacesItemModel::loadBookmarks() (placesitemmodel.cpp:628)
==6910== by 0x17CC34: PlacesItemModel::PlacesItemModel(QObject*) (placesitemmodel.cpp:66)
==6910== by 0x151779: PlacesItemModelTest::init() (placesitemmodeltest.cpp:238)
==6910== by 0x16AF5C: PlacesItemModelTest::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (placesitemmodeltest.moc:152)
==6910== by 0xC64C900: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (in /usr/lib64/libQt5Core.so.5.11.3)
==6910== by 0x4E526FB: ??? (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E530DC: ??? (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E53650: ??? (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E53B0A: QTest::qRun() (in /usr/lib64/libQt5Test.so.5.11.3)
==6910== by 0x4E53E2A: QTest::qExec(QObject*, int, char**) (in /usr/lib64/libQt5Test.so.5.11.3)
==6910==
My hunch is that `PlacesItemModel` uses the `DolphinPlacesModelSingleton::placesModel` as it's source model, and since that is a static variable it's state is carried across between tests. But I haven't figured out if this is indeed the problem, or if it is something else.
I really hope you can help clarify this mystery :)
REPOSITORY
R318 Dolphin
REVISION DETAIL
https://phabricator.kde.org/D20024
To: hallas, #dolphin, elvisangelaccio
Cc: kfm-devel, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190330/1eed9877/attachment.htm>
More information about the kfm-devel
mailing list