thread problem with dbus_connection_send_with_reply

[David Malcolm]

On Mon, 2004-11-01 at 23:40 -0500, Colin Walters wrote:
> Hi,
> 
> I have a multithreaded application.  One thread is queuing DBusMessages,
> and another thread simply runs the main loop.  I am very often seeing
> messages get lost; the notify functions for replies are never called,
> even though I'm pretty sure the replies have actually reached the
> application.
> 
> The sending thread code looks like this:
> 
>     g_assert (dbus_connection_send_with_reply (procdata->loader_connection,
> 					       message, &pendingcall,
> 					       1000 * 60));

So what happens if G_DISABLE_ASSERT is defined?


>     dbus_message_unref (message);
> 
>     notifydata = g_new0 (ImsepNotifyData, 1);
>     /* add stuff to notifydata */
> 
>     g_assert (dbus_pending_call_set_notify (pendingcall,
> 					    notify_image_completion,
> 					    notifydata,
> 					    (DBusFreeFunction) free_notify_data));

Ditto

> 
> 
> It seems to me that in between these two functions, the main loop could
> run and dispatch an incoming reply to the queued message before the
> pendingcall callback has been set.  (Also, isn't
> dbus_pending_call_set_notify itself thread-unsafe?  There's no locking
> for the notify function...)
> 
> The reason I bring this up here is because I don't really see an easy
> way to fix it, without making the send and pending_call_set_notify one
> atomic operation done under the connection lock.  This would bloat the
> dbus_connection_send_with_reply API; the above code would look like
> this:
> 
> g_assert (dbus_connection_send_with_reply (procdata->loader_connection,
> 					   message, &pendingcall,
>                                            notify_image_completion,
>                                            notifydata, (DBusFreeFunction) free_notify_data,
> 					   1000 * 60));

And thrice!

> 
> Actually, given all the comments in DBusPendingCall about thread safety,
> maybe it would be best to just suck it into DBusConnection entirely?  
> 
> Also, I think this bug is very related to (but not quite the same as):
> http://freedesktop.org/bugzilla/show_bug.cgi?id=857
> One approach for solving both problems might be for
> dbus_connection_send_with_reply_and_block to be written in terms of the
> above fixed dbus_connection_send_with_reply; it would create a
> DBusCondVar, and use an internal notify function that signaled the
> condition when the message was received.  Although, I guess that assumes
> the mainloop is running in a separate thread.  Hmm.  
> 
> 
>