<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="https://git.reviewboard.kde.org/r/119110/">https://git.reviewboard.kde.org/r/119110/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 4th, 2014, 7:22 p.m. UTC, <b>David Faure</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;">recursive mutexes are more expensive. Can you try this patch instead?
http://www.davidfaure.fr/2014/kprotocolmanager.cpp.diff</pre>
</blockquote>
<p>On July 4th, 2014, 7:27 p.m. UTC, <b>David Faure</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;">Actually, nothing beats a unittest :) Added, fix pushed.
http://commits.kde.org/kio/842bc8008f5eed03698e8ec67351e791b65ad2b1
I'll repack kio after dinner.</pre>
</blockquote>
<p>On July 4th, 2014, 9:02 p.m. UTC, <b>David Faure</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;">Done (v4.100.0-rc2). You can discard this RR.</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;">Just out of curiosity, what's so expensive about Recursive mutexes?</pre>
<br />
<p>- Vishesh</p>
<br />
<p>On July 4th, 2014, 9:58 p.m. UTC, Vishesh Handa wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and David Faure.</div>
<div>By Vishesh Handa.</div>
<p style="color: grey;"><i>Updated July 4, 2014, 9:58 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</div>
<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;"> KProtocolManager: Fix double mutex locking on a non recursive mutex
Currently called reparseConfiguration results in an infinite block
because the code tries to lock the mutex twice even though it is not a
recursive mutex.
Stack trace -
KProtocolManager::entryMap -> Tries to lock it
KIO::SlaveConfigPrivate::readGlobalConfig
KIO::SlaveConfig::reset
KProtocolManager::reparseConfiguration -> Holds the lock
This can easily be solved by making the mutex recursive, but that breaks
the asserts as they are using QMutex::tryLock to check if the mutex is
locked. With the mutex being recursive tryLock just results in them
getting locked again.
I'm not sure if this is the correct way of fixing it
Here is the full backtrace if anyone is interested -
#0 0x00007ffff328a359 in syscall () from /usr/lib/libc.so.6
#1 0x00007ffff3e21830 in _q_futex (addr=0x7ffff31a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>, op=0, val=3, timeout=0x0)
at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:154
#2 0x00007ffff3e21a04 in lockInternal_helper<false> (d_ptr=..., timeout=-1, elapsedTimer=0x0) at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:195
#3 0x00007ffff3e21891 in QBasicMutex::lockInternal (this=0x7ffff31a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>)
at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex_linux.cpp:211
#4 0x00007ffff3e2164c in QMutex::lock (this=0x7ffff31a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>)
at /home/vishesh/kde5/qt5/qtbase/src/corelib/thread/qmutex.cpp:225
#5 0x00007ffff2eed989 in QMutexLocker::QMutexLocker (this=0x7fffffffc6f0, m=0x7ffff31a4b00 <(anonymous namespace)::Q_QGS_kProtocolManagerPrivate::innerFunction()::holder>)
at /opt/qt5/include/QtCore/qmutex.h:136
#6 0x00007ffff2ee7f27 in KProtocolManager::entryMap (group=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:294
#7 0x00007ffff2ee5748 in KIO::SlaveConfigPrivate::readGlobalConfig (this=0x7b5d10) at /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:73
#8 0x00007ffff2ee5fa6 in KIO::SlaveConfig::reset (this=0x7b6b20) at /home/vishesh/kde5/src/frameworks/kio/src/core/slaveconfig.cpp:227
#9 0x00007ffff2ee7dfc in KProtocolManager::reparseConfiguration () at /home/vishesh/kde5/src/frameworks/kio/src/core/kprotocolmanager.cpp:278
#10 0x00007ffff2ea37ee in KIO::SessionData::reset (this=0x7b52f0) at /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:136
#11 0x00007ffff2ea3289 in KIO::SessionData::configDataFor (this=0x7b52f0, configData=..., proto=...) at /home/vishesh/kde5/src/frameworks/kio/src/core/sessiondata.cpp:102
#12 0x00007ffff2edb7f9 in KIO::SchedulerPrivate::metaDataFor (this=0x7b52d0, protocol=..., proxyList=..., url=...)
at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1049
#13 0x00007ffff2edc18b in KIO::SchedulerPrivate::setupSlave (this=0x7b52d0, slave=0x82d3d0, url=..., protocol=..., proxyList=..., newSlave=true, config=0x0)
at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1094
#14 0x00007ffff2edb737 in setupSlave (slave=0x82d3d0, url=..., protocol=..., proxyList=..., newSlave=true, config=0x0)
at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:1042
#15 0x00007ffff2eda759 in KIO::ProtoQueue::startAJob (this=0x7c11b0) at /home/vishesh/kde5/src/frameworks/kio/src/core/scheduler.cpp:621
#16 0x00007ffff2edd795 in KIO::ProtoQueue::qt_static_metacall (_o=0x7c11b0, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0x7fffffffcfa0)
at /home/vishesh/kde5/build/frameworks/kio/src/core/moc_scheduler_p.cpp:244
#17 0x00007ffff40879bf in QMetaObject::activate (sender=0x7c1208, signalOffset=3, local_signal_index=0, argv=0x0)
at /home/vishesh/kde5/qt5/qtbase/src/corelib/kernel/qobject.cpp:3680</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;">This can be tested by running `khotnewstuff "plasmoids.knsrc"` in knewstuff/tests/. Without this patch it just blocks.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/core/kprotocolmanager.cpp <span style="color: grey">(63baa6d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119110/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>