D14302: Don't block forever in ensureKdeinitRunning

2018-07-26 Thread Jaime Torres Amate
jtamate added a comment. Qt 5.11.0 and frameworks 5.46.0 over xcb. I'm sorry, I'll have to wait until the next lock, I didn't find the lock file neither the blocked kdeinit5. Is the complete name of the lock file be logged with qCDebug() ?. Just after killing dolphin yesterday afternoon

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure added a comment. In D14302#298099 , @thiago wrote: > In D14302#297515 , @dfaure wrote: > > > The idea of the old code was: if I can't get the lock, then someone else is already in the process

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Thiago Macieira
thiago added a comment. In D14302#297515 , @dfaure wrote: > The idea of the old code was: if I can't get the lock, then someone else is already in the process of starting kdeinit, so I'll just wait for that to happen, by locking again, i.e.

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R271:8097e1aa0d21: Dont block forever in ensureKdeinitRunning (authored by jtamate). REPOSITORY R271 KDBusAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14302?vs=38403=38448 REVISION

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. That's reasonable. Still when you see this warning in the future, it would be useful to find out what kdeinit+klauncher are actually doing, if klauncher doesn't manage to register with

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38403. jtamate edited the summary of this revision. jtamate added a comment. Don't wait forever, if the lock is blocked for more than 30 seconds, write a warning and don't try to start kdeinit5 again. REPOSITORY R271 KDBusAddons CHANGES SINCE LAST

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. The comments look good. The actual change, not so much ;) INLINE COMMENTS > kdeinitinterface.cpp:72 > +QProcess proc; > +// started asynchronously, so even if kdeinit5

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38387. jtamate edited the summary of this revision. jtamate added a comment. > I said from the start that it wasn't tryLock() that was blocking but lock(), good to see that this is now confirmed. I trusted blindly in gdb stacktrace, and also I didn't

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure added a comment. The idea of the old code was: if I can't get the lock, then someone else is already in the process of starting kdeinit, so I'll just wait for that to happen, by locking again, i.e. blocking on purpose, and then checking that the DBus name is up, i.e. the other

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38372. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. It is my first time of a backtrace that doesn't reflect the true state of a program. But I've learned the lesson, in case of

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added inline comments. INLINE COMMENTS > kdeinitinterface.cpp:46 > QLockFile lock(QDir::tempPath() + QLatin1Char('/') + > QLatin1String("startkdeinitlock")); > -if (!lock.tryLock()) { > +if (!lock.tryLock(timeout)) { > lock.lock(); This line doesn't need changing.

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment. In D14302#297159 , @jtamate wrote: > (gdb) disass > Dump of assembler code for function _ZN9QLockFile7tryLockEi: > 0x7f54be8bc752 <+2>: mov$0x,%eax > ... > 0x7f54be8bc76d

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment. Quickly checking on openSUSE Tumbleweed In D14302#297185 , @lvsouza wrote: > I think I know what is happening. This line > > QDeadlineTimer timer(qMax(timeout, -1));// QDT only takes -1 as "forever" > >

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Lamarque Souza
lvsouza added a comment. I think I know what is happening. This line QDeadlineTimer timer(qMax(timeout, -1));// QDT only takes -1 as "forever" passes the result of qMax() to QDeadlineTimer's constructor. That constructor receives a quint64. Since qMax() is a template:

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Jaime Torres Amate
jtamate added a comment. (gdb) disass Dump of assembler code for function _ZN9QLockFile7tryLockEi: 0x7f54be8bc750 <+0>: push %r15 0x7f54be8bc752 <+2>: mov$0x,%eax 0x7f54be8bc757 <+7>: mov$0x1,%edx 0x7f54be8bc75c

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment. > timer = {t1 = 9223372036854775807, t2 = 0, type = 1} This is indeed Forever. How did it get there? I showed in my debug session that the QDeadlineTimer is passed zero, and then it does initialise properly. So I'm now beginning to question the compiler.

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Jaime Torres Amate
jtamate added a comment. In D14302#297120 , @thiago wrote: > Can you print the contents of the timer object inside tryLock()? (gdb) info frame Stack level 2, frame at 0x7ffdda9967c0: rip = 0x7f54be8bc985 in

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment. Can you print the contents of the timer object inside tryLock()? REPOSITORY R271 KDBusAddons REVISION DETAIL https://phabricator.kde.org/D14302 To: jtamate, dfaure, #frameworks, thiago Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Thiago Macieira
thiago added a comment. In D14302#296671 , @jtamate wrote: > Another hypothesis: > > QDeadlineTimer::QDeadlineTimer(qint64 msecs, Qt::TimerType type) Q_DECL_NOTHROW > : t2(0) > { > setRemainingTime(msecs, type); > }

D14302: Don't block forever in ensureKdeinitRunning

2018-07-24 Thread Jaime Torres Amate
jtamate added a comment. Another hypothesis: QDeadlineTimer::QDeadlineTimer(qint64 msecs, Qt::TimerType type) Q_DECL_NOTHROW : t2(0) { setRemainingTime(msecs, type); } As t1 is not initialized and QDeadlineTimer::setPreciseRemainingTime only add to t1, it

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread Thiago Macieira
thiago added a comment. Quick testing via gdb: Breakpoint 1, QLockFile::tryLock (this=0x7fffc6d8, timeout=0) at /home/tjmaciei/src/qt/qt5/qtbase/src/corelib/io/qlockfile.cpp:241 241 Q_D(QLockFile); (gdb) n 242 QDeadlineTimer timer(qMax(timeout, -1));

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread Thiago Macieira
thiago added a comment. In D14302#296479 , @dfaure wrote: > I agree about tryLock(0) should return immediately, tryLock(-1) should block forever -- I wrote that code and that docu ;-) > > Thiago wrote QDeadLineTimer later on though, and

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread Jaime Torres Amate
jtamate added a comment. In D14302#296469 , @jtamate wrote: > I guess this is a Qt bug. > > qint64 QDeadlineTimer::remainingTime() const Q_DECL_NOTHROW > { > qint64 ns = remainingTimeNSecs(); > return ns <= 0 ? ns : (ns

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread David Faure
dfaure added a comment. I agree about tryLock(0) should return immediately, tryLock(-1) should block forever -- I wrote that code and that docu ;-) Thiago wrote QDeadLineTimer later on though, and ported QLockFile to it. Thiago, any input? REPOSITORY R271 KDBusAddons REVISION DETAIL

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread David Faure
dfaure added a reviewer: thiago. REPOSITORY R271 KDBusAddons REVISION DETAIL https://phabricator.kde.org/D14302 To: jtamate, dfaure, #frameworks, thiago Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread Jaime Torres Amate
jtamate added a comment. I guess this is a Qt bug. qint64 QDeadlineTimer::remainingTime() const Q_DECL_NOTHROW { qint64 ns = remainingTimeNSecs(); return ns <= 0 ? ns : (ns + 99) / (1000 * 1000); } Shouldn't it be: return ns <= 0 ? **0** : (ns +

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread Jaime Torres Amate
jtamate added a comment. I kept dolphin blocked, just in case. The backtrace: #0 0x7f54c47c4c90 in __GI___nanosleep (requested_time=requested_time@entry=0x7ffdda996710, remaining=remaining@entry=0x7ffdda996710) at ../sysdeps/unix/sysv/linux/nanosleep.c:28 #1

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Strange, I never had that bug. Note that you can use gdb to get a backtrace for a deadlock, no need for hotspot for that particular task. In fact, hotspot even misled you.

D14302: Don't block forever in ensureKdeinitRunning

2018-07-23 Thread Jaime Torres Amate
jtamate retitled this revision from "Don't block forever to ensureKdeinitRunning" to "Don't block forever in ensureKdeinitRunning". jtamate edited the summary of this revision. REPOSITORY R271 KDBusAddons REVISION DETAIL https://phabricator.kde.org/D14302 To: jtamate, dfaure, #frameworks

D14302: Don't block forever to ensureKdeinitRunning

2018-07-23 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Frameworks. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY My wife has been suffering this block for