Bug 311947

Summary: Crash when renaming folder to hidden if it has subfolders, expanded in panel
Product: [Applications] dolphin Reporter: Georgios Varisteas <yorgos.v>
Component: panels: foldersAssignee: Dolphin Bug Assignee <dolphin-bugs-null>
Status: RESOLVED FIXED    
Severity: crash Keywords: investigated
Priority: NOR    
Version: 2.1   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In: 4.11
Sentry Crash Report:

Description Georgios Varisteas 2012-12-19 18:45:52 UTC
Dolphin will crash when collapsing a folder tree in the panel, if previously...


Reproducible: Always

Steps to Reproduce:
0. make sure the folders panel is visible
1. assume folder tmp, enter
2. create subfolder tmp_s1, enter
3. create subfolder tmp_s2, expand the full tree in the panel:
tmp
  |_ tmp_s1
        |_ tmp_s2
4. prefix a '.' (dot) to tmp_s1's name to hide it. result:
tmp
        |_ tmp_s2
the indentation is not wrong and it's a first hint that something is wrong. tmp_s2 should disappear too.
5. collapse tmp.
Actual Results:  
4. the indentation is not wrong and it's a first hint that something is wrong. tmp_s2 should disappear too.
6. immediate crash

Expected Results:  
1. renaming should hide the whole subtree of tmp_s1, as if it never existed. 
2. Interacting with the panel after renaming should not lead to a crash (obviously)

Since this crash happens only when subfolders of the to-be-hidden folder are visible in the panel, a quick workaround is to allow the panel to include hidden folders.
Any rename operation other than the dot prefix will not lead to a crash.

Finally, I consider renaming a folder to be a major feature for a file manager.

Application: Dolphin (kdeinit4), signal: Segmentation fault
Using host libthread_db library "/lib64/libthread_db.so.1".
[Current thread is 1 (Thread 0x7ff9bda39780 (LWP 24704))]

Thread 2 (Thread 0x7ff9a3c7f700 (LWP 24707)):
#0  0x00007ff9baff3bc3 in poll () from /lib64/libc.so.6
#1  0x00007ff9b7d02936 in ?? () from /usr/lib64/libglib-2.0.so.0
#2  0x00007ff9b7d02a54 in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#3  0x00007ff9bc66d3f6 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#4  0x00007ff9bc63cbb2 in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#5  0x00007ff9bc63ce6d in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#6  0x00007ff9bc537d68 in QThread::exec() () from /usr/lib64/qt4/libQtCore.so.4
#7  0x00007ff9bc61b538 in ?? () from /usr/lib64/qt4/libQtCore.so.4
#8  0x00007ff9bc53a436 in ?? () from /usr/lib64/qt4/libQtCore.so.4
#9  0x00007ff9bc2a3da6 in start_thread () from /lib64/libpthread.so.0
#10 0x00007ff9baffcabd in clone () from /lib64/libc.so.6

Thread 1 (Thread 0x7ff9bda39780 (LWP 24704)):
[KCrash Handler]
#6  0x00007ff9bc5ebe18 in QUrl::path() const () from /usr/lib64/qt4/libQtCore.so.4
#7  0x00007ff9bcad6d80 in KUrl::path(KUrl::AdjustPathOption) const () from /usr/lib64/libkdecore.so.5
#8  0x00007ff9a922935b in KFileItemModel::expandedParentsCountCompare(KFileItemModel::ItemData const*, KFileItemModel::ItemData const*) const () from /usr/lib64/libdolphinprivate.so.4
#9  0x00007ff9a9229622 in KFileItemModel::lessThan(KFileItemModel::ItemData const*, KFileItemModel::ItemData const*) const () from /usr/lib64/libdolphinprivate.so.4
#10 0x00007ff9a9258d2f in KFileItemModelSortAlgorithm::merge(KFileItemModel*, QList<KFileItemModel::ItemData*>::iterator, QList<KFileItemModel::ItemData*>::iterator, QList<KFileItemModel::ItemData*>::iterator) () from /usr/lib64/libdolphinprivate.so.4
#11 0x00007ff9a9258df3 in KFileItemModelSortAlgorithm::sort(KFileItemModel*, QList<KFileItemModel::ItemData*>::iterator, QList<KFileItemModel::ItemData*>::iterator) () from /usr/lib64/libdolphinprivate.so.4
#12 0x00007ff9a9258dc8 in KFileItemModelSortAlgorithm::sort(KFileItemModel*, QList<KFileItemModel::ItemData*>::iterator, QList<KFileItemModel::ItemData*>::iterator) () from /usr/lib64/libdolphinprivate.so.4
#13 0x00007ff9a9258da8 in KFileItemModelSortAlgorithm::sort(KFileItemModel*, QList<KFileItemModel::ItemData*>::iterator, QList<KFileItemModel::ItemData*>::iterator) () from /usr/lib64/libdolphinprivate.so.4
#14 0x00007ff9a92263ee in KFileItemModel::removeItems(KFileItemList const&) () from /usr/lib64/libdolphinprivate.so.4
#15 0x00007ff9a9226fc8 in KFileItemModel::setExpanded(int, bool) () from /usr/lib64/libdolphinprivate.so.4
#16 0x00007ff9a92390de in KItemListController::mouseReleaseEvent(QGraphicsSceneMouseEvent*, QTransform const&) () from /usr/lib64/libdolphinprivate.so.4
#17 0x00007ff9a9236fb2 in KItemListController::processEvent(QEvent*, QTransform const&) () from /usr/lib64/libdolphinprivate.so.4
#18 0x00007ff9a923f023 in KItemListView::event(QEvent*) () from /usr/lib64/libdolphinprivate.so.4
#19 0x00007ff9bb79c4d4 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#20 0x00007ff9bb7a102d in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#21 0x00007ff9bd430da6 in KApplication::notify(QObject*, QEvent*) () from /usr/lib64/libkdeui.so.5
#22 0x00007ff9bc63de5c in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib64/qt4/libQtCore.so.4
#23 0x00007ff9bbd9708b in ?? () from /usr/lib64/qt4/libQtGui.so.4
#24 0x00007ff9bbd97d5b in ?? () from /usr/lib64/qt4/libQtGui.so.4
#25 0x00007ff9bbd987b5 in QGraphicsScene::mouseReleaseEvent(QGraphicsSceneMouseEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#26 0x00007ff9bbdab20f in QGraphicsScene::event(QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#27 0x00007ff9bb79c4d4 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#28 0x00007ff9bb7a102d in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#29 0x00007ff9bd430da6 in KApplication::notify(QObject*, QEvent*) () from /usr/lib64/libkdeui.so.5
#30 0x00007ff9bc63de5c in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib64/qt4/libQtCore.so.4
#31 0x00007ff9bbdc17ce in QGraphicsView::mouseReleaseEvent(QMouseEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#32 0x00007ff9bb7ee9ee in QWidget::event(QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#33 0x00007ff9bbb994a6 in QFrame::event(QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#34 0x00007ff9bbdc5d9b in QGraphicsView::viewportEvent(QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#35 0x00007ff9bc63dfe6 in QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () from /usr/lib64/qt4/libQtCore.so.4
#36 0x00007ff9bb79c4a1 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#37 0x00007ff9bb7a190d in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#38 0x00007ff9bd430da6 in KApplication::notify(QObject*, QEvent*) () from /usr/lib64/libkdeui.so.5
#39 0x00007ff9bc63de5c in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib64/qt4/libQtCore.so.4
#40 0x00007ff9bb79d4c5 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool) () from /usr/lib64/qt4/libQtGui.so.4
#41 0x00007ff9bb81c468 in ?? () from /usr/lib64/qt4/libQtGui.so.4
#42 0x00007ff9bb81ab19 in QApplication::x11ProcessEvent(_XEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#43 0x00007ff9bb842d3a in ?? () from /usr/lib64/qt4/libQtGui.so.4
#44 0x00007ff9b7d02653 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#45 0x00007ff9b7d02998 in ?? () from /usr/lib64/libglib-2.0.so.0
#46 0x00007ff9b7d02a54 in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#47 0x00007ff9bc66d39f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#48 0x00007ff9bb8429ee in ?? () from /usr/lib64/qt4/libQtGui.so.4
#49 0x00007ff9bc63cbb2 in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#50 0x00007ff9bc63ce6d in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#51 0x00007ff9bc6418eb in QCoreApplication::exec() () from /usr/lib64/qt4/libQtCore.so.4
#52 0x00007ff9a99f7853 in kdemain () from /usr/lib64/libkdeinit4_dolphin.so
#53 0x000000000040754c in _start ()
Comment 1 Frank Reininghaus 2012-12-19 19:04:01 UTC
Thanks for the report and for the precise steps, I could reproduce the crash. Might be related to bug 294616, bug 305283.
Comment 2 Frank Reininghaus 2013-01-05 13:16:03 UTC
After debugging this for a while, I think that I see what the problem is:

Normally, when a directory is deleted (or becomes invisible because it is renamed to something with a dot at the beginning), KFileItemModel receives the itemsDeleted signal from KDirLister, tries to find all children of that directory, and removes everything from the model (and hence, also from the view).

However, in the described use case, the following happens:

1. KDirLister tells us via its refreshItems signal that the name of "tmp_s1/tmp_s2" has changed to ".tmp_s1/tmp_s2". It does not tell us about the name change of "tmp_s1" itself, though, because...

2. KDirLister then tells us that "tmp_s1" has been deleted because it is invisible now. KFileItemModel tries to find its children, but cannot find any because technically, "tmp_s1" does not look like the parent of ".tmp_s1/tmp_s2". (To be precise: KFileItemModel doesn't look at the names at all to determine the children, it just takes all items *after* the removed one which have a greater expansion level as children, but because of the renaming, ".tmp_s1/tmp_s2" now appears *before* the removed "tmp_s1".)

Now we have an item which has no valid parent -> trying to dereference the dangling "parent" causes a crash.

I see two possibilities to fix this:

a) Prevent that KDirLister emits the refreshItems signal for an item whose parent is reported as "deleted".

b) Move the entire "filter hidden files" logic into KFileItemModel.

The second one is not straightforward to implement, not sure about the first one.
Comment 3 Frank Reininghaus 2013-01-05 13:19:29 UTC
Just for the record: the reason why this only happens when renaming via the Folders Panel is that DolphinView::slotRoleEditingFinished changes the URL of the renamed item inside the model *before* actually renaming the directory. Therefore, the 'itemsDeleted' signal for the old name does not have any effect there.
Comment 4 Georgios Varisteas 2013-01-05 14:13:33 UTC
Hi Frank,

I'm not sure what you mean by "renaming via the Folders Panel" but in my case I rename inline using the main part of dolphin. Never actually tried renaming through the panel.

However, dolphin crashes only when the folder panel is visible and the subtree expanded (parentless children as you explained why in a previous post).

cheers
Comment 5 Frank Reininghaus 2013-02-10 17:14:12 UTC
Git commit b80384caae111e5c9145ada293922acafe5fa085 by Frank Reininghaus.
Committed on 10/02/2013 at 18:09.
Pushed by freininghaus into branch 'master'.

Re-organize the code that compares expanded items

The previous approach, which was based on comparing the URLs as
strings, was not only very complex, but also could lead to
inconsistencies in the model, namely, that not all children were
removed from the model when the dir lister reported the parent as
deleted. Later on, this could even lead to a crash.
FIXED-IN: 4.11
REVIEW: 108766

M  +29   -87   dolphin/src/kitemviews/kfileitemmodel.cpp
M  +0    -16   dolphin/src/kitemviews/kfileitemmodel.h
M  +52   -0    dolphin/src/tests/kfileitemmodeltest.cpp

http://commits.kde.org/kde-baseapps/b80384caae111e5c9145ada293922acafe5fa085
Comment 6 Frank Reininghaus 2013-02-10 17:18:48 UTC
I've found a way to fix the problem. Unfortunately, it required rewriting much of the code that determines in which order the expanded children of a folder appear in the view. The new code is much simpler and also faster, but it's a very large change, and I think that it is therefore not suitable for a bug fix release in the KDE 4.10 series. We have seen much smaller patches cause ugly regressions, so I feel that it's better that this new code gets thorough testing. Therefore, the fix is only in the master branch, which will become KDE 4.11.

Thanks again for your help and the detailed and precise steps to reproduce the crash. Excellent bug reports like this one really make our lives easier :-)