Bug 14581 - Calling out to user code should be re-entrant
Summary: Calling out to user code should be re-entrant
Status: RESOLVED MOVED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on: 36205
Blocks: 16668
  Show dependency treegraph
 
Reported: 2008-02-20 03:48 UTC by Simon McVittie
Modified: 2018-08-22 22:02 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2008-02-20 03:48:07 UTC
telepathy-glib has code that re-enters the main loop (much like gtk_dialog_run()) in order to make "blocking" method calls without suffering the undesirable side-effects of making a blocking call in libdbus (message re-ordering, and no other messages or input being processed).

I ran into trouble implementing the following pattern: when method A returns, do something with its return value and pass it straight to method B. The problematic case was when method A was called asynchronously, and method B was called in a "blocking" way by re-entering the main loop.

To make this work, I had to ensure that user callbacks (specifically, the callback for method A) were run from a high priority idle handler, rather than having dbus_connection_dispatch() in the stack - this is because the D-Bus event source is not marked as re-entrant, so when the main loop is re-entered, the D-Bus event source is blocked and the reply to method B never arrives.

The same thing happens if I make a re-entrant call to method B in a handler for D-Bus signal C (this is an obvious and convenient way to implement "whenever C happens, call B").

If it is in fact safe to use the D-Bus event source in a re-entrant way, it should be marked as such so GLib won't block it. If it's *not* safe to do so, the "high priority idle handler" hack should probably move into dbus-glib, so higher-level code like telepathy-glib doesn't have to worry about it.
Comment 1 Martin Pitt 2008-07-15 05:28:16 UTC
This painfully hit me as well while trying to implement a PackageKit client class with dbus-python (see bug 16668).

Simon, do you have some details what you did to replace the inner gobject main loop which would work around this bug? Thanks in advance!
Comment 2 Martin Pitt 2008-07-24 07:50:27 UTC
BTW, bug 16668 has a test case for this bug.
Comment 3 Will Thompson 2010-06-02 08:19:40 UTC
Taking a look at the message dispatch GSource in dbus-glib and at dbus_connection_dispatch(), I think the latter's documentation — which claims that it's re-entrant if and only if you've initialized threads with recursive mutex functions — is correct, and we could call g_source_set_can_recurse() appropriately if we could check the status of libdbus's threads handling.

The default mutex implementations in libdbus are re-entrant with pthreads, and not on win32. :(

Would it be excessively unpleasant to add dbus_threads_check_reentrant() or similar to glib? Then applications which call dbus_g_thread_init() and are on posix-y platforms could at least not suffer from this bug.
Comment 4 Havoc Pennington 2010-06-29 13:53:09 UTC
Maybe adding dbus_threads_has_recursive_mutex() makes sense, though it would just be a nice thing for anyone installing their own thread functions, which is probably nobody.

Otherwise we can just assume dbus_threads_init_default() was used and that it has recursive mutexes, so just remove the limitation on dbus-glib mainloop source (assuming things work if you do so, I haven't tested it or anything).

If recursive locks don't work on windows port the windows port should be fixed.
Comment 5 Ralf Habacker 2011-07-01 14:41:20 UTC
(In reply to comment #3)
> Taking a look at the message dispatch GSource in dbus-glib and at
> dbus_connection_dispatch(), I think the latter's documentation — which claims
> that it's re-entrant if and only if you've initialized threads with recursive
> mutex functions — is correct, and we could call g_source_set_can_recurse()
> appropriately if we could check the status of libdbus's threads handling.
> 
> The default mutex implementations in libdbus are re-entrant with pthreads, and
> not on win32. :(
> 
Can you give some more backgrounds of this assumption ? 

The CreateMutex doc at http://msdn.microsoft.com/en-us/library/ms682411%28v=vs.85%29.aspx says: 
"... The thread that owns a mutex can specify the same mutex in repeated wait function calls without blocking its execution. Typically, you would not wait repeatedly for the same mutex, but this mechanism prevents a thread from deadlocking itself while waiting for a mutex that it already owns. However, to release its ownership, the thread must call ReleaseMutex once for each time that the mutex satisfied a wait. ...

This looks like recursive behavior to me.
Comment 6 Simon McVittie 2011-07-18 10:04:57 UTC
(In reply to comment #5)
> (In reply to comment #3)
> > The default mutex implementations in libdbus are re-entrant with pthreads, and
> > not on win32. :(
> > 
> Can you give some more backgrounds of this assumption ?

It's dbus Bug #36204, please see there.
Comment 7 GitLab Migration User 2018-08-22 22:02:54 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus-glib/issues/2.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.