Bug 261541 - Applications don't exit properly due to KDirWatch
Summary: Applications don't exit properly due to KDirWatch
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: David Faure
URL:
Keywords: investigated
: 261463 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-29 15:33 UTC by Benni Hill
Modified: 2011-01-24 12:31 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
gdb backtraces (7.79 KB, text/plain)
2010-12-29 15:33 UTC, Benni Hill
Details
workaround (1.35 KB, patch)
2011-01-03 14:47 UTC, Benni Hill
Details
Check for QCoreApplication instance before deleting QFSWatcher. (571 bytes, patch)
2011-01-12 03:21 UTC, Benni Hill
Details
Or just disable QFSWatcher when we have inotify (374 bytes, patch)
2011-01-12 04:45 UTC, Benni Hill
Details
deleting QFSWatcher with qAddPostRoutine (1.75 KB, patch)
2011-01-14 21:49 UTC, Benni Hill
Details
patch against current trunk (971 bytes, patch)
2011-01-18 13:45 UTC, Benni Hill
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Benni Hill 2010-12-29 15:33:44 UTC
Created attachment 55350 [details]
gdb backtraces

Version:           unspecified (using KDE 4.5.90) 
OS:                Linux

When I start certain applications (tested with kate, dolphin and kdevelop) their procceses keep running even after closing the main window.
I attached backtraces of the mentioned applications and it seems to me as a problem of KBookmarkManager.

Reproducible: Always

Steps to Reproduce:
Start e.g. dolphin, hit Ctrl+Q to quit and do a "ps x | grep dolphin". The process is still running.

It's not possible to restart dolphin until the hanging process is killed.
Comment 1 Benni Hill 2010-12-30 10:55:05 UTC
Maybe this bug is related to these qt bugs:
http://bugreports.qt.nokia.com/browse/QTBUG-10030
http://bugreports.qt.nokia.com/browse/QTBUG-15255
Comment 2 Benni Hill 2010-12-30 16:16:11 UTC
In kfilebookmarkhandler.cpp the KBookmarkManager instance doesn't get deleted in the destructor. Might that be the problem?
Comment 3 Benni Hill 2011-01-02 16:43:15 UTC
Another idea:
In kfileplacesmodel.cpp two KBookmarkManagers are created but never deleted (lines 95 and 175, the latter through KFilePlacesSharedBookmarks' constructor).
Comment 4 Benni Hill 2011-01-03 14:34:20 UTC
Please ignore my last two comments.

I think I isolated the bug to a problem in QFileSystemWatcher. When trying to add "/etc/security/fileshare.conf" (which is not readable on my system for my user) QFileSystemWatcher hangs on exit, causing this error during run time:

QInotifyFileSystemWatcherEngine::addPaths: inotify_add_watch failed: Permission denied
QFileSystemWatcher: failed to add paths: /etc/security/fileshare.conf

This problem seems to be solved through this commit in KSambaShare (/etc/security/fileshare.conf has been removed) so the above mentioned steps to reproduce no longer work:
http://websvn.kde.org/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp?r1=928071&r2=1209710

But in KFileSharePrivate it still exists. To reproduce start dolphin, go to file->properties, close the dialog and exit dolphin. The process is still running unless /etc/security/fileshare.conf is readable by the current user.
Comment 5 Benni Hill 2011-01-03 14:47:25 UTC
Created attachment 55518 [details]
workaround

I created this patch as a workaround for the problem.
Comment 6 Benni Hill 2011-01-08 13:51:58 UTC
I can still reproduce this bug as described in comment 0 on KDE SC 4.6 RC2 (Kubuntu 10.10).
Comment 7 Frank Reininghaus 2011-01-09 09:20:09 UTC
Thanks for the bug report and the detailed analysis!

@David: can you have a look at the patch?
Comment 8 Benni Hill 2011-01-11 23:16:06 UTC
I think I figured out why the apps only hang when "/etc/security/fileshare.conf" is not readable:
On Linux KDirWatch uses INotify as default. But when a file is not readable, useINotify(Entry* e) fails and KDirWatch tries to use QFileSystemWatcher (see kdirwatch.cpp lines 913 and following). Then on app-exit QFileSystemWatcher hangs due to the bugs in Qt I mentioned in comment 1.

When I set m_preferredMethod to KDirWatch::QFSWatch, KDirWatch will always hang on exit.
Comment 9 Benni Hill 2011-01-12 03:21:20 UTC
Created attachment 55893 [details]
Check for QCoreApplication instance before deleting QFSWatcher.

This patch solves the problem for me.
It's just another workaround but I think if QCoreApplication is not running any longer QFSWatcher will be removed from memory nevertheless. What do you think?
Comment 10 Benni Hill 2011-01-12 04:45:23 UTC
Created attachment 55895 [details]
Or just disable QFSWatcher when we have inotify
Comment 11 David Faure 2011-01-13 21:40:47 UTC
Hello, thanks for the investigations.

The patch in comment #10 breaks the idea that the kdirwatch backend can be chosen at runtime, with the key [DirWatch] PreferredMethod=QFSWatch for instance.

The patch in comment #9 reads like "let's leak memory", which doesn't look too nice.
Yes the system will reclaim memory soon afterwards, but it means anyone debugging memory leaks (e.g. with valgrind leakcheck) will see that one pop up.
I see that the Qt "bug" has been fixed by a documentation change only, apparently we do have to delete the fswatcher before QCoreApplication... We should look into using qAddPostRoutine to delete it while qapp still exists.
Comment 12 Benni Hill 2011-01-14 21:49:41 UTC
Created attachment 56027 [details]
deleting QFSWatcher with qAddPostRoutine

OK, I understand.
What do you think about this patch?
Is it necessary to delete all entries before deleting QFSWatcher?
Comment 13 Patrizio Bassi 2011-01-16 11:49:27 UTC
*** Bug 261463 has been marked as a duplicate of this bug. ***
Comment 14 Patrizio Bassi 2011-01-16 11:51:28 UTC
i used patch from comment #12 and it works ok for me.

in my opinion this is a critical bug. maybe it's better to add this patch for 4.6.0 final and write a better WA for 4.6.1 than leaving the final release with this huge usability issue.
Comment 15 David Faure 2011-01-17 01:21:23 UTC
SVN commit 1214952 by dfaure:

Apply hotfix for bug 261541 (apps hang on exit due to KDirWatch+QFileSystemWatcher),
while working on a better solution.
CCBUG: 261541


 M  +2 -0      kdirwatch.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1214952
Comment 16 David Faure 2011-01-17 01:40:29 UTC
SVN commit 1214954 by dfaure:

Found how to reproduce the qfswatch hang bug in the unittest.
Fixed by adding a qAddPostRoutine which destroys KDirWatch::sef() while qApp is still around.
If this proves stable in trunk, let's backport it to the 4.6 branch
CCBUG: 261541


 M  +16 -1     io/kdirwatch.cpp  
 M  +11 -0     tests/kdirwatch_unittest.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1214954
Comment 17 David Faure 2011-01-17 01:46:22 UTC
(In reply to comment #12)
Thanks for the patch. It was incorrect (it would share the qfsWatch instance between all KDirWatch instances, and any KDirWatch instance other than the first one, wouldn't get connected to the qfsWatch) (-- yes this is a bit tricky, KDirWatch can be used as a singleton but other instances can also be created).
However it reminded me on how to use qAddPostRoutine, and it confirmed that the approach works. New patch committed to trunk, please test and remind me to backport if everything seems good.

Overall I still think this is a Qt bug in the first place, but well, at least we can work around it.
Comment 18 Frank Reininghaus 2011-01-17 15:33:12 UTC
@David: Since your commit 1214954, Dolphin trunk crashes every time I close it:

#8  0x00007ffec2723d68 in qt_message_output (msgType=QtFatalMsg, buf=
    0x7ea4b8 "Fatal Error: Accessed global static 'KDirWatch *s_pKDirWatchSelf()' after destruction. Defined at /home/kde-devel/kde/src/KDE/kdelibs/kdecore/io/kdirwatch.cpp:1722")
    at global/qglobal.cpp:2282
#9  0x00007ffec2723ee4 in qt_message(QtMsgType, const char *, typedef __va_list_tag __va_list_tag *) (msgType=QtFatalMsg, msg=
    0x7ffec2e620e0 "Fatal Error: Accessed global static '%s *%s()' after destruction. Defined at %s:%d", ap=0x7fffd792d500) at global/qglobal.cpp:2328
#10 0x00007ffec2724752 in qFatal (msg=0x7ffec2e620e0 "Fatal Error: Accessed global static '%s *%s()' after destruction. Defined at %s:%d") at global/qglobal.cpp:2511
#11 0x00007ffec2d1a609 in operator-> (this=0x7ffec3127cd0) at /home/kde-devel/kde/src/KDE/kdelibs/kdecore/io/kdirwatch.cpp:1722
#12 0x00007ffec2d1a5aa in operator KDirWatch* (this=0x7ffec3127cd0) at /home/kde-devel/kde/src/KDE/kdelibs/kdecore/io/kdirwatch.cpp:1722
#13 0x00007ffec2d1a738 in KDirWatch::self () at /home/kde-devel/kde/src/KDE/kdelibs/kdecore/io/kdirwatch.cpp:1725
#14 0x00007ffec57b28f6 in KNFSShare::~KNFSShare (this=0x848a80, __in_chrg=<value optimized out>) at /home/kde-devel/kde/src/KDE/kdelibs/kio/kio/knfsshare.cpp:178
#15 0x00007ffec57b2972 in KNFSShare::~KNFSShare (this=0x848a80, __in_chrg=<value optimized out>) at /home/kde-devel/kde/src/KDE/kdelibs/kio/kio/knfsshare.cpp:181
#16 0x00007ffec57b2cbb in destroy () at /home/kde-devel/kde/src/KDE/kdelibs/kio/kio/knfsshare.cpp:217
#17 0x00007ffec56d836b in KCleanUpGlobalStatic::~KCleanUpGlobalStatic (this=0x7ffec5aff3f8, __in_chrg=<value optimized out>) at /home/kde-devel/kde/src/KDE/kdelibs/kdecore/kernel/kglobal.h:62
#18 0x00007ffec0ce04e1 in __run_exit_handlers () from /lib64/libc.so.6
#19 0x00007ffec0ce0535 in exit () from /lib64/libc.so.6
#20 0x00007ffec0cc9b84 in __libc_start_main () from /lib64/libc.so.6
#21 0x0000000000400859 in _start () at ../sysdeps/x86_64/elf/start.S:113
Comment 19 Benni Hill 2011-01-18 13:45:15 UTC
Created attachment 56166 [details]
patch against current trunk

Dolphin also crashes here.
So, another try.
Comment 20 David Faure 2011-01-18 19:38:50 UTC
Oh, interesting. I just fixed the crash in trunk, by commenting out the unnecessary use of KDirWatch in KNFSShare, but I'm seeing your suggestion now, and it looks good, it will prevent other problems with the use of KDirWatch::self in destructors of singletons. I'll commit it, thanks!
Comment 21 David Faure 2011-01-18 19:42:59 UTC
SVN commit 1215435 by dfaure:

Apply better fix for the use of KDirWatch::self() in the destructor of a singleton:
 make it possible, by not destroying the KDirWatch early, only its qfswatcher.
Patch by alter schwede, thanks!
CCBUG: 261541


 M  +10 -4     kdirwatch.cpp  
 M  +2 -0      kdirwatch.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1215435
Comment 22 David Faure 2011-01-24 12:31:18 UTC
Seems to be stable now, but not much point in backporting (let's just "leak" the fswatcher on app exit in 4.6 branch, it won't hurt anyone) -> closing.