Bug 59780

Summary:	on service crash, dbus daemon sends timeout messages in wrong order
Product:	dbus	Reporter:	Allison Lortie (desrt) <desrt>
Component:	core	Assignee:	D-Bus Maintainers <dbus>
Status:	RESOLVED MOVED	QA Contact:	D-Bus Maintainers <dbus>
Severity:	normal
Priority:	medium	CC:	chengwei.yang.cn, msniko14
Version:	1.5	Keywords:	love
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Allison Lortie (desrt) 2013-01-23 23:00:38 UTC

If you write a service in a serial way (ie: non-threaded) then by the usual well-ordering guarantees of D-Bus, all users of that service can assume that they will receive replies to their method calls in the same order that they sent the method calls.

One exception to this case is when the service crashes.  In that case, D-Bus sends error messages back to the client _in reverse order_.

See https://bugzilla.gnome.org/show_bug.cgi?id=687120 for a report of an assertion failure caused by the above assumption being violated by this behaviour.

Probably it would be relatively easy to fix this.

Comment 1 Simon McVittie 2013-01-24 12:40:43 UTC

(In reply to comment #0)
> If you write a service in a serial way (ie: non-threaded) then by the usual
> well-ordering guarantees of D-Bus, all users of that service can assume that
> they will receive replies to their method calls in the same order that they
> sent the method calls.

Slight nitpicking: if those methods' implementations are synchronous (reply before processing any other D-Bus messages), yes; if those methods' implementations queue up work to be done later (even in the same thread), not true.

> One exception to this case is when the service crashes.  In that case, D-Bus
> sends error messages back to the client _in reverse order_.

That does sound wrong.

> Probably it would be relatively easy to fix this.

I can't spend a great deal of time on D-Bus right now, but I'd be happy to review patches. bus_connection_drop_pending_replies() is probably the place to start. It appears that bus_expire_list_add_link() prepends to the list, meaning that traversal in natural order deals with the most recently-added first: changing bus_expire_list_add_link() from prepend to append might be enough.

I'd like to have a regression test for this before changing anything. test/dbus-daemon.c would be a good place to add it: use of GLib utility code in that test is encouraged. The right setup would probably be something like this:

* set a filter on the "right connection" which (wrongly) drops method-call messages on the floor
* use the "left connection" to call that method several times
* make the "right connection" disconnect
* check that the "left connection" gets the timeout errors in the expected order

Comment 2 Simon McVittie 2014-01-17 16:12:07 UTC

This shouldn't be rocket science to fix or regression-test, if someone is interested. I'd be happy to review a patch.

Comment 3 GitLab Migration User 2018-10-12 21:15:22 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/issues/77.

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.