Bug 379254

I have the same Issue since a week or more.
Everything seems to be working, even Bluetooth, The Chrome Apps im Using and everything about my Touchpad.
The Only issue is, that the login needs really much tiome, and based on the lower Errors, ive getting three error popups about missing, chrome, blueman-applet and Synaptik.



sadako@Yukie:~$ grep 'failed to launch' ~/.xsession-errors 
ksmserver: autostart service "/home/sadako/.config/autostart/B8375615B423566EB75F241DC788C2C0BFE8AF2C.google-chrome-service.desktop" failed to launch
ksmserver: autostart service "/home/sadako/.config/autostart/synaptiks.desktop" failed to launch
ksmserver: autostart service "/etc/xdg/autostart/blueman.desktop" failed to launch

CCing Luboš as I think he's the only person who worked on ksmserver between .4 and .5.

It appears to me that da44dd6eae8b9ef4161680efcb9319a7267fe77e which changed autostarts from QProcess to KRun causes this.

I have seen this failure for a desktop file with non-existent binary, which supposedly isn't the problem of comment #0 where perhaps it's a dbus problem (krun would wait for x-dbus-servicenames to be actually registered on the bus, if that doesn't happen after 30 seconds it will abort waiting and throw an error dialog IIRC).

Super terrible regressions.

Concerning KRun,
I reported also : https://bugs.kde.org/show_bug.cgi?id=379254

there is a chance that they could be related...

sorry wrong type in previous message the correct url:
https://bugs.kde.org/show_bug.cgi?id=379192

>krun would wait for x-dbus-servicenames to be actually registered on the bus

Yes, but not KRun::runApplication which is what's used in this case.

I still don't understand the point of the port (It was to fix %u  characters in the exec line...but it's autolaunched, so why?), but I also don't really see what's wrong with the new code.

Worst case we revert it back to what I deliberatey wrote in the first place, then maybe do a QProcess -> KProcess conversion to fix whatever Lubos was trying to fix in the first place.

I see the bug.

It's the error dialogs
‎They're blocking, and it's a new window so kwin will ask ksmserver for restoration details. 

Will do the thing I just said ^

I cannot reproduce (openSUSE42.2 + 5.9.5 packages), nor I can quite see why this should break (apparently KRun::runApplication() works elsewhere, so why not here).

What exactly do the error dialogs say?

"‎They're blocking, and it's a new window so kwin will ask ksmserver for restoration details." - This doesn't look like the cause of the problem, nor it looks like something that would be much of a problem.

Created attachment 105222 [details]
example desktop file

as expected, having any unavailable binary in the Exec ought to trigger this. 

if you are still having trouble reproducing: 

- install neon user edition in a VM and update via discover http://files.kde.org/neon/images/neon-useredition/current/
- supposedly a wayland session in docker should also work https://community.kde.org/Neon/Docker

KRun::runApplication can spawn several error dialogs. My theory was these blocked ksmserver.

All Qt apps (including kwin/plasmashell) make blocking calls to the X session manager on startup.  
If KSMServer is ever blocked that app simply won't start.

However, it seems that the nested event loop in the dialog does mean ksmserver still continues to work as a session manager. So my reasonining is not entirely right.

In any case, we don't want to ever prevent autostarting other apps in the event of an error anyway, so it needs changing regardless.

Git commit ea3f87c5df0251838da71c473fd7b790c932d8b0 by David Edmundson.
Committed on 27/04/2017 at 14:53.
Pushed by davidedmundson into branch 'Plasma/5.8'.

Revert "launch autostart apps in ksmserver using KRun"

KRun::runApplication will show blocking error dialogs if it fails to
find the executable
This means we don't autostart the next app, which could be fatal if it
comes before...

Summary:
...kwin/plasma
We shouldn't be having blocking calls in ksmserver it can deadlock
And even in the best case we'd still end up blocking ksplash for 30
seconds

We then port to KProcess which was part of the motivation behind the
patch as it
has better stdout handling

This reverts commit 0f19e92f3e85d064de9cebf280fa8e085485c2e0.

Also added port of autostarting applications to KProcess
It has better stdout handling

Test Plan: Logged in, still got my main session

Reviewers: #plasma, mart

Reviewed By: mart

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D5618

M  +14   -3    ksmserver/startup.cpp

https://commits.kde.org/plasma-workspace/ea3f87c5df0251838da71c473fd7b790c932d8b0

(In reply to Lubos Lunak from comment #7)
> I cannot reproduce (openSUSE42.2 + 5.9.5 packages), nor I can quite see why
> this should break (apparently KRun::runApplication() works elsewhere, so why
> not here).
> 
> What exactly do the error dialogs say?
> 
> "‎They're blocking, and it's a new window so kwin will ask ksmserver for
> restoration details." - This doesn't look like the cause of the problem, nor
> it looks like something that would be much of a problem.

The messages are like that: Dialog title is 'Sorry' and the message is like

Could not find program 'lib/x86_64-linux-gnu/libexec/kdeconnectd'

It is interesting that kwrited 5.9.4 (Neon) package contains file

/etc/xdg/autostart/kwrited-autostart.desktop

but not 4:5.9.5-0neon+16.04+build33  version:

10:51% dpkg -L kwrited                                     
/.
/usr
/usr/lib
/usr/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu/qt5
/usr/lib/x86_64-linux-gnu/qt5/plugins
/usr/lib/x86_64-linux-gnu/qt5/plugins/kf5
/usr/lib/x86_64-linux-gnu/qt5/plugins/kf5/kded
/usr/lib/x86_64-linux-gnu/qt5/plugins/kf5/kded/kwrited.so
/usr/share
/usr/share/knotifications5
/usr/share/knotifications5/kwrited.notifyrc
/usr/share/doc
/usr/share/doc/kwrited
/usr/share/doc/kwrited/changelog.Debian.gz
/usr/share/doc/kwrited/copyright
/usr/share/lintian
/usr/share/lintian/overrides
/usr/share/lintian/overrides/kwrited

For other packages, e.g. kdeconnectd, /etc/xdg/autostart/kdeconnectd.desktop is there. But it specifies


Exec=lib/x86_64-linux-gnu/libexec/kdeconnectd

(the exact location is /usr/lib/x86_64-linux-gnu/libexec/kdeconnectd)

Maybe path lookup is not done properly?

> 
> For other packages, e.g. kdeconnectd, /etc/xdg/autostart/kdeconnectd.desktop
> is there. But it specifies
> 
> 
> Exec=lib/x86_64-linux-gnu/libexec/kdeconnectd
> 
> (the exact location is /usr/lib/x86_64-linux-gnu/libexec/kdeconnectd)
> 
> Maybe path lookup is not done properly?

I have remove 'mpd', after that there was only one error - about 'kdeconnectd'. Then I edited /etc/xdg/autostart/kdeconnectd.desktop and specified the full path,

Exec=/usr/lib/x86_64-linux-gnu/libexec/kdeconnectd

After that login works fine, without any delays and error popups. So it seems that the new ksmserver does not process Exec=lib/x86_64-linux-gnu/libexec/kdeconnectd properly (I am not sure how it should work - trying to prepend '/' and '/usr/' to Exec line?).

(In reply to David Edmundson from comment #10)
> Differential Revision: https://phabricator.kde.org/D5618
> 
> M  +14   -3    ksmserver/startup.cpp
> 
> https://commits.kde.org/plasma-workspace/
> ea3f87c5df0251838da71c473fd7b790c932d8b0

Seems that 5.9.5 needs more than just that one commit reverting to fix. 

Now fails to build in Neon, KDE and Kubuntu CI with.

[ 41%] Building CXX object ksmserver/logout-greeter/CMakeFiles/ksmserver-logout-greeter.dir/main.cpp.o
cd /<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/obj-x86_64-linux-gnu/ksmserver/logout-greeter && /usr/bin/c++   -DKCOREADDONS_LIB -DKGUIADDONS_LIB -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_DISABLE_DEPRECATED_BEFORE=0 -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_NO_URL_CAST_FROM_STRING -DQT_PRINTSUPPORT_LIB -DQT_QML_LIB -DQT_QUICK_LIB -DQT_WIDGETS_LIB -DQT_X11EXTRAS_LIB -DQT_XML_LIB -DTRANSLATION_DOMAIN=\"ksmserver\" -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/obj-x86_64-linux-gnu/ksmserver/logout-greeter -I/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/ksmserver/logout-greeter -I/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/obj-x86_64-linux-gnu -I/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/obj-x86_64-linux-gnu/ksmserver -I/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/libkworkspace -I/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/obj-x86_64-linux-gnu/libkworkspace -I/usr/include/phonon4qt5 -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++-64 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtQuick -isystem /usr/include/x86_64-linux-gnu/qt5/QtQml -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtX11Extras -isystem /usr/include/KF5/KDeclarative -isystem /usr/include/KF5 -isystem /usr/include/KF5/KConfigCore -isystem /usr/include/KF5/KPackage -isystem /usr/include/KF5/KCoreAddons -isystem /usr/include/KF5/KIconThemes -isystem /usr/include/KF5/KI18n -isystem /usr/include/KF5/KDELibs4Support -isystem /usr/include/KF5/KDELibs4Support/KDE -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -isystem /usr/include/x86_64-linux-gnu/qt5/QtPrintSupport -isystem /usr/include/KF5/KCrash -isystem /usr/include/KF5/KWidgetsAddons -isystem /usr/include/KF5/KConfigWidgets -isystem /usr/include/KF5/KCodecs -isystem /usr/include/KF5/KConfigGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtXml -isystem /usr/include/KF5/KAuth -isystem /usr/include/KF5/KIOCore -isystem /usr/include/KF5/KService -isystem /usr/include/KF5/KIOFileWidgets -isystem /usr/include/KF5/KIOWidgets -isystem /usr/include/KF5/KJobWidgets -isystem /usr/include/KF5/KCompletion -isystem /usr/include/KF5/KBookmarks -isystem /usr/include/KF5/KItemViews -isystem /usr/include/KF5/KXmlGui -isystem /usr/include/KF5/Solid -isystem /usr/include/KF5/KNotifications -isystem /usr/include/KF5/KWindowSystem -isystem /usr/include/KF5/KGuiAddons -isystem /usr/include/KF5/KUnitConversion -isystem /usr/include/KF5/KTextWidgets -isystem /usr/include/KF5/SonnetUi -isystem /usr/include/KF5/KParts -isystem /usr/include/KF5/KWayland/Client  -g -O2 -fdebug-prefix-map=/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -Wdate-time -D_FORTIFY_SOURCE=2 -std=c++0x -fno-operator-names -fno-exceptions -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type -Wvla -Wdate-time -fvisibility=hidden -fvisibility-inlines-hidden   -fPIC -std=gnu++11 -o CMakeFiles/ksmserver-logout-greeter.dir/main.cpp.o -c /<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/ksmserver/logout-greeter/main.cpp
/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/ksmserver/startup.cpp: In member function ‘void KSMServer::slotAutoStart()’:
/<<BUILDDIR>>/plasma-workspace-5.9.5+p17.10+git20170427.1658/ksmserver/startup.cpp:682:26: error: ‘DesktopExecParser’ is not a member of ‘KIO’
         auto arguments = KIO::DesktopExecParser(service, QList<QUrl>()).resultingArguments();

Given comments above, I find it rather clear that the actual problem is with the broken .desktop files (in one distro, possibly, kdeconnectd doesn't have such a problem on openSUSE). So it could be argued that there indeed should be a dialog about the error (although I'm not strictly opposed to reverting it in the branches).

plasma-workspace 5.9.5.1 tar available with this revert.
https://www.kde.org/info/plasma-5.9.5.php

(In reply to Lubos Lunak from comment #14)
> Given comments above, I find it rather clear that the actual problem is with
> the broken .desktop files (in one distro, possibly, kdeconnectd doesn't have
> such a problem on openSUSE). So it could be argued that there indeed should
> be a dialog about the error (although I'm not strictly opposed to reverting
> it in the branches).

I think youre right.

I fixed my errors by removing all of the desktop files from the kde Autostart Folder, and the blueman-applet desktop file from the etc/xdg/autostart folder.
it seems, that there where some old desktop files, that are not needed anymore.

Now everything runs fine like before.
:)

So I've intentionally broken one of my local autostart .desktop files and that does cause the problem, and it's indeed somewhat annoying that the whole startup is blocked by it. And, looking at KRun, this is caused by a dialog blocking the whole ksmserver in the autostart code.

So I see basically 3 viable possibilities:
- Leave master as it is. And I don't think this is such a bad choice, there are not supposed to be broken .desktop files after all.
- Add some kind of "QString* error" argument to the KRun call, which would prevent KRun from showing the dialog itself and let ksmserver show it in a non-blocking way.
- Make ksmserver have klauncher do the actual launching, like it was before (i.e. a DBus call to "start_service" or somesuch). But I don't know what was the exact idea with moving the autostart code to ksmserver.

I intentionally didn't list "revert my commits in master". The code in KRun (or klauncher) does way more than just get the executable path from the .desktop file and launch it. I don't find it a good idea to either copy&paste a lof of code to ksmserver, or to trade a blatantly obvious problem (that's not even our problem) for a possible bunch of non-obvious problems.

TBH any and all errors during autostart should *at most* be thrown on stdout/stderr. Any visual information about it would need a wall of text to not freak the user out. The problem is that throwing a dialog saying "firekitten failed to launch" lacks any sort of context, so unless you provide context by explaining what that means (and that can mean any number of things from broken desktop file to crash) the user reaction will be equally varied from worried about something being broken to annoyed by getting notifications about things they do not even care about.

On top of that the assumption that there must not be bad desktop files is simply wrong. On a distro level, perhaps, beyond that it's the wild west. XDG_CONFIG_HOME/autostart can get littered into by any app at any time without the user giving consent or actively needing to know about it, so the chance of there being desktop files that do not actually have an Exec is exceptionally high. Say the user gets a third party dropbox tray application in a tar, they untar it and run the binary, the binary may well install itself to be autostarted at this point. When the user removes the dropbox binary to "uninstall" it, the desktop file ends up broken. The user did not know about the desktop file and neither should they need to.

With that in mind what should happen is the krun `qstring *error` approach and then issue a qWarning() if there is an error. If anyone thinks this deserves UI backing the way to go about this IMHO is to record failures in some file and then have the autostart KCM use that information to show a warning and relevant information for failing entries. If the user consciously expects an autostart and it doesn't happen the user would go check the autostart is correctly set up in the KCM, and there the error has context and the user has means to do something about whatever is wrong with it.

Simple by default, powerful when needed.

(FTR: when I looked at the autostart code a couple months back I did not see a reason why it would need KRun to do its job. The way I see it all it does is add overhead compared to using KProcess directly)

As this bugreport shows, throwing such errors to stderr means that nobody will notice them, and then distros will happily ship broken apps (that said, I've noticed that kcm_autostart does not one let do anything with global autostart files, not even show them, so if a distro manages to do this, the user won't have an easy way to handle the problem).

KRun does a number of things that KProcess-based code doesn't (KDesktopExecParser, KAuthorized, CWD, DiscreteGPU). If they wouldn't be needed for launching .desktop files, then there would be no point in having them in KRun either. And it seems lame to require every place that wants to run a .desktop file to do this manually.

I guess it'll be better to move this discussion to kde-core-devel.

As I have said, the problem aren't distributions, they are third party developers/apps. In either case the user is neither the distribution nor the third party developer, so you'd be messaging the wrong party if you'd throw a visible error.

Relevant list is plasma-devel mind you.