<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/5193/">http://reviewboard.kde.org/r/5193/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 31st, 2010, 9:27 a.m., <b>Mario Bensi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">sorry about this problem.
The problem here, it's the QFileSystemWatcher must be deleted before Q[Core]Application, your solution is good but i think the QFileSystemWatcher is leaked when you detach this from the parent. I send another patch which complete your solution.
Thanks for your help.</pre>
</blockquote>
<p>On August 31st, 2010, 9:39 a.m., <b>Mario Bensi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">fix commited revision 1170242</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"but i think the QFileSystemWatcher is leaked when you detach this from the parent."
it's on application exit, so the process will terminate shortly. the OS cleans up at that point, so there is no leak. the reason i took the orphaning approach is that any such activity at application exit may end up (now or in the future) being undefined and lead to a repeat of the same or similar type of bug; by simply letting the kernel clean up once the process has terminated, there is less possibility of regression in future due to changes in the internal implementations of QCoreApplication and/or QThread infrastructure. in any case, thanks for committing (though i could have done that as well after you marked it with "ship it" :), i'll close this review request now.</pre>
<br />
<p>- Aaron</p>
<br />
<p>On August 31st, 2010, 1:09 a.m., Aaron Seigo wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Solid.</div>
<div>By Aaron Seigo.</div>
<p style="color: grey;"><i>Updated 2010-08-31 01:09:50</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">we're seeing this hang on Solid-using apps on application exit:
#0 0xffffe424 in __kernel_vsyscall ()
#1 0xb5e9b125 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
#2 0xb5f28f69 in QWaitConditionPrivate::wait (this=0x84116d8, time=4294967295)
at /home/aseigo/kde4/qt/src/corelib/thread/qwaitcondition_unix.cpp:88
#3 0xb5f28d6d in QWaitCondition::wait (this=0x8428190, mutex=0x8428178, time=4294967295)
at /home/aseigo/kde4/qt/src/corelib/thread/qwaitcondition_unix.cpp:160
#4 0xb5f2864e in QThread::wait (this=0x83bfac8, time=4294967295)
at /home/aseigo/kde4/qt/src/corelib/thread/qthread_unix.cpp:660
#5 0xb6004859 in QFileSystemWatcher::~QFileSystemWatcher (this=0x8411200,
__in_chrg=<value optimized out>) at /home/aseigo/kde4/qt/src/corelib/io/qfilesystemwatcher.cpp:440
#6 0xb60049ad in QFileSystemWatcher::~QFileSystemWatcher (this=0x8411200,
__in_chrg=<value optimized out>) at /home/aseigo/kde4/qt/src/corelib/io/qfilesystemwatcher.cpp:456
#7 0xb6049478 in QObjectPrivate::deleteChildren (this=0x83c0d70)
at /home/aseigo/kde4/qt/src/corelib/kernel/qobject.cpp:1949
#8 0xb6047d44 in QObject::~QObject (this=0x83c1618, __in_chrg=<value optimized out>)
at /home/aseigo/kde4/qt/src/corelib/kernel/qobject.cpp:945
#9 0xb4e883ea in Solid::Ifaces::DeviceManager::~DeviceManager (this=0x83c1618,
__in_chrg=<value optimized out>)
at /home/aseigo/kde4/KDE/kdelibs/solid/solid/ifaces/devicemanager.cpp:29
#10 0xb4ece7a1 in Solid::Backends::Fstab::FstabManager::~FstabManager (this=0x83c1618,
__in_chrg=<value optimized out>)
at /home/aseigo/kde4/KDE/kdelibs/solid/solid/backends/fstab/fstabmanager.cpp:132
#11 0xb4ece7db in Solid::Backends::Fstab::FstabManager::~FstabManager (this=0x83c1618,
__in_chrg=<value optimized out>)
at /home/aseigo/kde4/KDE/kdelibs/solid/solid/backends/fstab/fstabmanager.cpp:134
#12 0xb4e70430 in qDeleteAll<QList<QObject*>::const_iterator> (begin=..., end=...)
at /home/aseigo/kde4/qt-copy/include/QtCore/qalgorithms.h:322
#13 0xb4e702d9 in qDeleteAll<QList<QObject*> > (c=...)
at /home/aseigo/kde4/qt-copy/include/QtCore/qalgorithms.h:330
#14 0xb4e6fe8d in Solid::ManagerBasePrivate::~ManagerBasePrivate (this=0x8416c58,
__in_chrg=<value optimized out>) at /home/aseigo/kde4/KDE/kdelibs/solid/solid/managerbase.cpp:47
#15 0xb4e72f72 in Solid::DeviceManagerPrivate::~DeviceManagerPrivate (this=0x8416c50,
__in_chrg=<value optimized out>) at /home/aseigo/kde4/KDE/kdelibs/solid/solid/devicemanager.cpp:48
#16 0xb4e72fc1 in Solid::DeviceManagerPrivate::~DeviceManagerPrivate (this=0x8416c50,
__in_chrg=<value optimized out>) at /home/aseigo/kde4/KDE/kdelibs/solid/solid/devicemanager.cpp:62
#17 0xb4e763f2 in QThreadStorage<Solid::DeviceManagerPrivate*>::deleteData (x=0x8416c50)
at /home/aseigo/kde4/qt-copy/include/QtCore/qthreadstorage.h:133
#18 0xb5f26466 in QThreadStorageData::finish (p=0x80528d8)
at /home/aseigo/kde4/qt/src/corelib/thread/qthreadstorage.cpp:185
#19 0xb6030be6 in QCoreApplicationPrivate::~QCoreApplicationPrivate (this=0x8079940,
__in_chrg=<value optimized out>)
at /home/aseigo/kde4/qt/src/corelib/kernel/qcoreapplication.cpp:291
#20 0xb53a446b in QApplicationPrivate::~QApplicationPrivate (this=0x8079940,
it's hanging in a call to QInotifyFileSystemWatcherEngine::wait(), which is actually a call to QThread::wait(). on my laptop, this results in the process not exiting fully, a spike in CPU usage that slowly (over the course of several minutes) comes down to ~1% of the CPU. the spike happens increasingly quickly, and the spin down takes increasingly long as it approaches 0, which is probably a clue. :)
as can be seen in the backtrace, QThreadStorageData::finish is being called, which results in Solid being destroyed, which results in the QThread-based QFileSystemWatcherEngine going into a wait mode ... deadlock.
this patch, while certainly not "pretty", does the job.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">run plasma-desktop, try kquitapp plasma-desktop and ensure it exits cleanly.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=249557">249557</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdelibs/solid/solid/backends/fstab/fstabmanager.h <span style="color: grey">(1170183)</span></li>
<li>/trunk/KDE/kdelibs/solid/solid/backends/fstab/fstabmanager.cpp <span style="color: grey">(1170183)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/5193/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>