dbus_connection_dispatch() in dbus-connection.c makes a copy of the filter list
before calling the filter callback but it does not check whether the called
filter callback removed any callbacks from the (original) filter list using
Maybe it could be fixed as follows:
1) Make a copy of the filter list (as before).
2) Call a callback in the list copy.
3) Remove the called callback from the list copy.
4) Remove all callbacks that do no longer occur in the original list from the
5) If the list copy still has one or more functions, go to step 2.
What do you think about this?
The usual way this problem is fixed is to have a way to mark the removed
callback as destroyed, then check for the destroyed marker on each callback
prior to invoking it from the copied list.
The easiest way to mark destroyed is probably to set filter->function = NULL
when the filter is removed.
Actually I think that we can only fix the case when the callback calls
dbus_connection_remove_filter() to remove a filter function. I don't see how we
could fix the case when the callback calls dbus_bus_remove_match(), because a
message matching the match could be already in the client buffers. Or maybe I'm
I think in order to fix the dbus_bus_remove_match() case, the client library
would need to discard all messages matching the removed match from the buffers.
(In reply to comment #1)
> The usual way this problem is fixed is to have a way to mark the removed
> callback as destroyed, then check for the destroyed marker on each callback
> prior to invoking it from the copied list.
> The easiest way to mark destroyed is probably to set filter->function = NULL
> when the filter is removed.
That seems to be already done, I just need to add the checking. I'll make a
patch for this. (But fixing this bug will require more as Comment #2 says.)
I think there is also another bug in the original code:
_dbus_message_filter_unref() is called for the filter list copy, but that will
call the user callback, which is not probably what the user expects...
Created attachment 6882 [details] [review]
proposed partial fix
My previous comment turned out to be a lie, but here is a patch that fixes a
possible crash, unless I have more brain damages.
looks correct, thanks.
Actually it was just a partial fix :) But I discussed this bug with some
colleagues and realised that this cannot be fixed, because clearing up the
client buffer would require knowledge about _all_ the matches, not just the one
that was removed. So, this is a WONTFIX.