Bug 90312

Created attachment 115540 [details] [review]
A patch that extends lock's range in live_messages_notify().

live_messages_notify() is called with the following stack trace:

#0  _dbus_transport_ref (transport=transport@entry=0x114d0)
#1  live_messages_notify (counter=<optimized out>, user_data=0x114d0)
#2  free_counter (element=0x117d8, data=0xb5b02eb0)
#3  dbus_message_cache_or_finalize (message=0xb5b02eb0)
#4  dbus_message_unref (message=0xb5b02eb0)
#5  external_function()

None of the above functions locks the connection with _dbus_connection_lock(), while live_messages_notify() refs and unrefs the transport.

Another thread may send something at the same time using same transport:
#0  _dbus_transport_unref (transport=0x114d0)
#1  _dbus_connection_do_iteration_unlocked (connection=0x11b28, ...)
#2  _dbus_connection_send_preallocated_unlocked_no_update (connection=0x11b28, ...)
#3  0xb62e5992 in _dbus_connection_send_unlocked_no_update (connection=0x11b28, message=0xb5b030e0, ...)
#4  0xb62e6092 in dbus_connection_send_with_reply (connection=0x11b28, message=0xb5b030e0, ...)
#5  0xb62e6244 in dbus_connection_send_with_reply_and_block (connection=0x11b28, message=0xb5b030e0, ...)
#6  another_external_function()

This results in the following state:
  Id   Target Id         Frame 
* 2    Thread 0xb54ff460 "tt" _dbus_transport_unref (transport=0x114d0) at dbus-transport.c:504
  1    Thread 0xb56ff460 "tt" _dbus_transport_ref (transport=0x114d0) at dbus-transport.c:486

Then, with non-atomic refcount, it sometimes crashes.

A simple solution for this is to protect _dbus_transport_ref() and _dbus_transport_unref() with connection lock.

Such lock is already used in live_messages_notify(), but only to protect call to (*transport->vtable->live_messages_changed). Nothing more really exists in live_messages_notify(), so I think lock range can be extended to protect also ref and unref calls.

This is what the attached patch does.

Test scenario is almost the same as in bug 89297. However, call to live_messages_notify() has to be triggered somehow (e.g. counter->size_value has to reach counter->notify_size_guard_value).