-
-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
auto-start m-n-d with session #214
Conversation
Disclaimer: I have never experienced the problem, so take this with a grain of salt. Should we really not have the service file? It seems like an expectation many have, and in "most" cases of a system well installed, it should work fine. |
Personal, i never run in this issue because kde isn't installed. But starting the daemon via autostart plus using dbus-activation doesn't make any sense. Well as i said before this is a proof-of-concept which i want to test for myself. |
This is the question...... |
My concern is basically if some people rely on MND as their
It can be tested just the same: if it happens anyway, sending a notification on a setup showing the bug should try to start the KDE implementation even with this change. |
@cwendling Of course can i do another PR with auto-starting the daemon and dbus-activation, but for me it's a double functionality and not needed when the daemon is running. Edit: This wiki describes the problems when several implementation of freedesktop-notification services are installed. |
I forgot to say this PR solves the issue in my test VM when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builds fine with these changes, and works as intended both on x11 and on wayfire started from a tty.
EDIT: also works fine in wayfire started from SDDM, so works in all use cases I am set up to test
8a02eb9
to
a7e1acf
Compare
I banged my head and found a valid reason for myself to keep dbus-activation. For example when the daemon is killed by a faulty notification application the debus-activation can restart the daemon. |
The D-Bus service file MUST be removed as part of this, otherwise you're not actually fixing the problem. |
What about making the idle exit conditional instead? I've done that there: https://github.com/mate-desktop/mate-notification-daemon/commits/start-with-session |
I still think it's less intrusive and less likely to break anybody's working setup if we keep D-Bus activation, but if causes too much trouble I don't care -- so long as a regular MATE session works, that's fine with me in the end. |
Well, it still fixes the problem MATE user sees. But maybe we're not good citizen if we keep it -- yet, AFAICT only GNOME doesn't provide it for its notification implementation (?). |
Before the last update i tried to use |
See link to archwiki. |
I am fine with dbus-activation because it can restart the daemon. |
I suggest you give my linked branch a try then. WFM nicely, with the idle exit feature when launched on-demand, and staying forever otherwise. |
I did it first, but other maintainer wasn't happy with it. |
OK, this suggests that actually KDE shouldn't have a problem here, as its daemon is always running? It also shows a solution to select which daemon is activated with D-Bus activation. |
I tested current state in VM when kde-daemon is installed. Mnd works fine. |
This can still cause conflicts even in DEs with built-in implementations (GNOME and KDE), as it introduces the opening for a race condition during DE startup. The ONLY safe way for multiple services providing the same service name to coexist is for none of them to use service activation and for each to be directly started by its respective DE. |
kde plasma 5 (f38) use dbus-activation too. |
You linked a non existing PR :) |
@raveit65 oops, I meant to link to the branch… see commit there: https://github.com/mate-desktop/mate-notification-daemon/commits/start-with-session. Anyway, it's easy enough to test locally: $ git fetch origin start-with-session
$ git checkout --track origin/start-with-session
$ make |
@cwendling
Second terminal:
Honestly, i prefer my current solution. |
Fair enough 😁
I really don't understand what could cause my take to be worse than yours particularly on Wayland, it has zero code specific to one or the other; and if you run it without arguments it should behave the same. Could you get a backtrace so we can see what the actual issue is? While I don't particularly care if we drop the D-Bus activation entirely given we have fixed all cases that could be problematic (and while we should fix any crash in the daemon itself, not being able to restart it automatically if it does crash is not great) beyond what I already mentioned, but I would really prefer that if we are D-Bus-activated we do idle exit, because it's actually one of the main advantages activation has: the software can come and go as needed, not wasting resources when unused. [edit] PS: I won't be able to test anything Wayland in the near future, if we go that route I'll have to setup a VM for this, but I won't have time before October at the earliest… |
Only a minimal stacktrace because my cable landline is broken today and i am connected with my cell phone, so i can't download huge debuginfo packages.
Maybe @lukefromdc is more lucky to test this under wayfire. |
Wait, you edited your backtrace, which it totally different now!? Anyway, this does not seem at all related to my changes vs. yours, can you verify this indeed works flawlessly with yours!?? Anyway, what about this (for your pre-edit backtrace): diff --git a/src/daemon/daemon.c b/src/daemon/daemon.c
index fd5d3f3..a9b63f5 100644
--- a/src/daemon/daemon.c
+++ b/src/daemon/daemon.c
@@ -1140,6 +1140,7 @@ static gboolean screensaver_active(GtkWidget* nw)
if (proxy == NULL) {
g_warning("Failed to get dbus connection: %s", error->message);
g_error_free (error);
+ return active;
}
variant = g_dbus_proxy_call_sync (proxy, |
Installing and then uninstalling all of KDE on my bare metal setup would be a bandwidth and a cleanup problem, but about to test this on wayfire to see if I can duplicate that crash |
It is the same crash , i only installed the local m-n-d-debuginfo package.
|
It didn't look the same, as it was thread1 that was crashing then, rather than the "gdbus" one. Anyway, please try my one-line patch, it should help some -- not saying it would fix it all though. |
I always rebuild an rpm so i need a patch. I can later generate a patch for my self when i found time. ....i need a timeout :) |
This just worked fine on my end, though restarting by calling the daemon after terminating it works only on Xorg. Will have to try it with an SDDM wayfire session, and all four themes to try and duplicate the crash |
@lukefromdc |
@raveit65 ah, you realized (my turn! :D) |
NOT sure, I used the main branch of this PR and it is getting confusing |
OK, git gui was able to find https://github.com/mate-desktop/mate-notification-daemon/commits/start-with-session and so long as I have my wayland session autostarting the daemon it works fine there with the latest changes (no crash). Note that "enable do not disturb" must be turned OFF for notifications to show even though the tray applet (a status icon) is not used in wayland |
Yeah, the branch from my PR works great. |
That one worked too on my end with the 3ed commit applied |
@cwendling Beside from this i am thinking why we should remove dbus-activation when all other DEs except gnome gives a shit about? |
This allows to run as a regular session service that does not exit, but also as a well-behaved D-Bus-activated service. Make the default behavior not to exit, but when activated through D-Bus.
Relying on D-Bus activation to launch org.freedesktop.Notifications can result in selecting the wrong implementation if multiple daemons are installed. Fix this by launching m-n-d with the session rather than depending on D-Bus activation. We keep D-Bus activation for the moment for backward compatibility and to re-start the daemon in otherwise non-problematic situations if it crashes. Fixes #133 and #174.
a7e1acf
to
fc6a01d
Compare
If it's laziness… I just hijacked your PR not to make a 3rd one scattering discussion.
Yeah, that was a weird bug… probably nobody ever had a situation where the m-n-d dbus service worked but not the screensaver I suppose.
Great, thanks for testing again. |
This is a proof-of-concept rework of an very old PR from 2013.
#10
It removes dbus-activation for notification-daemon and the daemon will start via autostart folder in x11 and wayland session.
Relying on D-Bus activation to launch org.freedesktop.Notifications can result in a race condition if multiple daemons are present on one computer, as explained further in the commit messages.
For example, GNOME 2.x vs KDE:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=584859
https://bugs.kde.org/show_bug.cgi?id=212656
And KDE vs. Unity (notify-osd):
http://ubuntuforums.org/showthread.php?t=1973315
These are just examples. The general problem is discussed further here:
https://bugzilla.redhat.com/show_bug.cgi?id=484945
We have several reports about conflict with kde-notification when both enviroments are installed.
This PR should fix the issues.
I like to test this for myself a while because i am not sure about the consequences.
In wayfire session the daemon can be started via autostart-command in wayfire.ini.