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 dbus_bus_remove_match(). 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 list copy. 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 confused?
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.
Committed, 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.
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.