> On July 4, 2014, 7:22 p.m., David Faure wrote: > > recursive mutexes are more expensive. Can you try this patch instead? > > http://www.davidfaure.fr/2014/kprotocolmanager.cpp.diff
Actually, nothing beats a unittest :) Added, fix pushed. http://commits.kde.org/kio/842bc8008f5eed03698e8ec67351e791b65ad2b1 I'll repack kio after dinner. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119110/#review61614 ----------------------------------------------------------- On July 4, 2014, 9:40 a.m., Vishesh Handa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119110/ > ----------------------------------------------------------- > > (Updated July 4, 2014, 9:40 a.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kio > > > Description > ------- > > 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 > > > Diffs > ----- > > src/core/kprotocolmanager.cpp 63baa6d > > Diff: https://git.reviewboard.kde.org/r/119110/diff/ > > > Testing > ------- > > This can be tested by running `khotnewstuff "plasmoids.knsrc"` in > knewstuff/tests/. Without this patch it just blocks. > > > Thanks, > > Vishesh Handa > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
