D14302: Don't block forever in ensureKdeinitRunning

[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 takes forever to start, the lock will be released
> +    proc.start(srv, args);

But that means the other processes coming into this method, will either succeed in  tryLock() and will run yet another kdeinit instance (your system is slow and you're bombarding it with kdeinit processes trying to start at the same time?),
or worse, tryLock() fails and lock() succeeds (because the first process released the lock too early here), and then isServiceRegistered fails (because we're trying too early), and again we start yet another kdeinit process that will fight it with the currently starting one.

If the bug is that kdeinit takes forever to start (not just a long time) and QProcess::execute never returns, then that's the actual bug. This proposed change is just a workaround which potentially makes things worse (10 kdeinit processes attempting to start at the same time).

BTW ~QProcess would kill kdeinit, what you meant was startDetached. But all of the above is the reasoning against startDetached, actually ;)

REPOSITORY
  R271 KDBusAddons

REVISION DETAIL
  https://phabricator.kde.org/D14302

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180725/281dc602/attachment-0001.html>