Bug 94155 - Make _dbus_connection_get_next_client_serial() public
Summary: Make _dbus_connection_get_next_client_serial() public
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Ralf Habacker
QA Contact: D-Bus Maintainers
Depends on:
Reported: 2016-02-15 11:07 UTC by Ralf Habacker
Modified: 2018-10-12 21:26 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

sequence diagram showing recent state (23.06 KB, image/png)
2016-02-15 11:07 UTC, Ralf Habacker
sequence diagram with fixed serial access (25.16 KB, image/png)
2016-02-15 11:08 UTC, Ralf Habacker

Description Ralf Habacker 2016-02-15 11:07:47 UTC
Created attachment 121759 [details]
sequence diagram showing recent state

While working on https://bugreports.qt.io/browse/QTBUG-44490, which provides access to 'serial' and 'reply serial to' in QtDBus, an issue getting the message serial in multi threaded raised up.

With recent dbus implementation the message serial is only available *after* sending the dbus message with one of the dbus_connection_send... functions.

In QtDBus the message is constructed in one thread and send from another. Dumping the message serial in the first thread immediatly after triggering send may return 0, because the message has not been send yet. (see sequence diagram 'current-state.png')

From my observations a solution to this issue (and may be similar in multithreaded environments) would be by constructing a dbus message including a reserved serial. This could be done by making _dbus_connection_get_next_client_serial() public. (see with-client_serial.png)
Comment 1 Ralf Habacker 2016-02-15 11:08:28 UTC
Created attachment 121760 [details]
sequence diagram with fixed serial access
Comment 2 Simon McVittie 2016-02-24 08:09:52 UTC
(In reply to Ralf Habacker from comment #0)
> From my observations a solution to this issue (and may be similar in
> multithreaded environments) would be by constructing a dbus message
> including a reserved serial. This could be done by making
> _dbus_connection_get_next_client_serial() public. (see
> with-client_serial.png)

_dbus_connection_get_next_client_serial() needs to be called under the DBusConnection's lock, so a public wrapper around it would need to take and release the lock. If you're posting the DBusMessage to another thread to be sent, then you were probably trying to avoid lock contention on the DBusConnection anyway?

This would make the serial numbers non-sequential (a thread could allocate a new serial number, let's say 23, and then not use it until after messages with serial numbers 24 and 25 had already been sent). Is that something we're OK with happening? In principle the serial numbers are basically opaque tokens (and kdbus calls its equivalent a cookie, not a serial number, to emphasize this) but applications might be relying on the thing called a serial number to be, well, serial.

IMO the fact that dbus_connection_send() updates the actual message's serial number is an implementation detail, and I'm not really comfortable with a binding like QtDBus relying on it (particularly across threads). The correct way to get the serial number of the message you just sent, if you need that, is to get it from dbus_connection_send() or one of the similar functions. QtDBus presumably posts an event containing the message from a user thread to some sort of D-Bus thread; could it post an event back that means "I have sent your message and its serial number is n", if that's desired functionality?

I would prefer not to encourage use of dbus_message_set_serial(): at the moment anyone using that API is probably wrong (unless they're doing something very odd, like serializing messages to disk, and probably even then).

I wonder whether it might be better to preallocate a serial number in dbus_connection_preallocate_send() (really _dbus_connection_preallocate_send_unlocked() which is already under the lock), have a new public function dbus_connection_preallocated_send_get_serial(), and stamp it on the message in _dbus_connection_send_preallocated_unlocked_no_update()? That still makes serial numbers non-sequential, and it still results in DBusConnection lock contention between the user thread and the D-Bus thread, but it has the right feeling of "serial numbers on outgoing messages are a property of the connection or the send-operation, not the message" IMO.

From the Qt bug, it looks as though you introduced this feature for debugging (to correlate sent messages with dbus-monitor). Is there any other reason to want this feature in Qt? It seems like a lot of effort to go to for that. Messages are totally-ordered within DBusConnection, and the QtDBus message-sending thread presumably pops messages off its queue in the same order they were pushed; so if you know the order in which messages were queued for sending (which you can know by making the threads that queued them write to a log that is ordered, for example stderr) you know the order in which they will be sent.
Comment 3 GitLab Migration User 2018-10-12 21:26:35 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/145.

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.