GNOME Bugzilla – Bug 641411
gdesktopappinfo signals lost if it's the session bus connection owner
Last modified: 2011-02-08 04:37:55 UTC
<mhr3> walters, one more thing about the Launched signal, there must be some threading issue, most of the time i dont see the signal in dbus-monitor, if i'm stepping the app in gdb i do <mhr3> walters, yep, unreffing connection means any queued messages will be discarded, as it calls worker_stop, which in turn calls cancel on the worker's cancellable So this bug will happen if we happen to be the first thing to get a connection to the session bus. The only thing I can think of is to add g_dbus_connection_flush_if_last_reference() which would be horrible to implement.
I suppose the easier way to fix this is reffing the connection, sending the signal using g_dbus_connection_send_message_with_reply and unref it in the AsyncReadyCallback.
(In reply to comment #1) > I suppose the easier way to fix this is reffing the connection, sending the > signal using g_dbus_connection_send_message_with_reply and unref it in the > AsyncReadyCallback. Hmm. I'm not sure; there's no actual reply for a signal, and I'm not sure that function would then call back when the message is simply flushed.
I believe g_dbus_connection_flush() is what you want.
Created attachment 180026 [details] [review] Proposed patch I just tested with this patch, works like charm.
(In reply to comment #4) > Created an attachment (id=180026) [details] [review] > Proposed patch > > I just tested with this patch, works like charm. This patch is wrong - you will never get a reply to a signal! It probably only works because the "reply" is never received. Instead, please try this g_dbus_connection_flush_sync (session_bus, NULL, NULL); _right_ after g_dbus_connection_send_message(). Does that work? (Related: Why is notify_desktop_launch() using the low-level API and not something like g_dbus_connection_emit_signal() ?!?)
(In reply to comment #5) > (In reply to comment #4) > > Created an attachment (id=180026) [details] [review] [details] [review] > > Proposed patch > > > > I just tested with this patch, works like charm. > > This patch is wrong - you will never get a reply to a signal! It probably only > works because the "reply" is never received. > > Instead, please try this > > g_dbus_connection_flush_sync (session_bus, NULL, NULL); > > _right_ after g_dbus_connection_send_message(). Does that work? Btw, if that works (which I believe it should), the next step is to do it async instead of blocking the calling thread. Actually, the easiest way to do this is g_dbus_connection_flush (session_bus, NULL, /* GCancellable */ NULL, /* GAsyncReadyCallback */ NULL); /* user_data */ and you're done! It's that easy because async methods take a ref on connection (for the duration of the async call) and you are allowed to pass NULL for your callback! So there! :-)
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Created an attachment (id=180026) [details] [review] [details] [review] [details] [review] > > > Proposed patch > > > > > > I just tested with this patch, works like charm. > > > > This patch is wrong - you will never get a reply to a signal! It probably only > > works because the "reply" is never received. It's actually called after the timeout, so yea... not too good. > > > > Instead, please try this > > > > g_dbus_connection_flush_sync (session_bus, NULL, NULL); > > > > _right_ after g_dbus_connection_send_message(). Does that work? > > Btw, if that works (which I believe it should), the next step is to do it async > instead of blocking the calling thread. Actually, the easiest way to do this is > > g_dbus_connection_flush (session_bus, > NULL, /* GCancellable */ > NULL, /* GAsyncReadyCallback */ > NULL); /* user_data */ > > and you're done! It's that easy because async methods take a ref on connection > (for the duration of the async call) and you are allowed to pass NULL for your > callback! So there! :-) Yep, that works fine as well.
> Yep, that works fine as well. OK, thinking more about it we probably want to do the flush_sync() (rather than the async flush()) to avoid possible problems with the process exiting before the connection has been flushed.
(In reply to comment #8) > > Yep, that works fine as well. > > OK, thinking more about it we probably want to do the flush_sync() (rather than > the async flush()) to avoid possible problems with the process exiting before > the connection has been flushed. We decided earlier to take the hit if the process exits. We don't want to flush_sync here since that means that the GDBus using process gets blocked here; and gnome-shell does run through this code.
Created attachment 180111 [details] [review] gdesktopappinfo: Asynchronously flush after sending notification If we were the initial connection owner, unref will destroy the connection immediately, and we may lose messages. Asynchronously flush to avoid that.
(In reply to comment #6) > Btw, if that works (which I believe it should), the next step is to do it async > instead of blocking the calling thread. Actually, the easiest way to do this is > > g_dbus_connection_flush (session_bus, > NULL, /* GCancellable */ > NULL, /* GAsyncReadyCallback */ > NULL); /* user_data */ > > and you're done! It's that easy because async methods take a ref on connection > (for the duration of the async call) and you are allowed to pass NULL for your > callback! So there! :-) I attached a patch which does exactly this.