Passing a rule with a destination= field to BecomeMonitor() doesn't match anything. For example, dbus-monitor destination=org.freedesktop.Notifications doesn't print any calls on the notifications interface, but dbus-monitor without match rules does. Calling AddMatch() with the same rule (and eavesdrop=true) works as expected.
Are these messages actually sent to the destination "org.freedesktop.Notifications", or are they in fact sent to something like ":1.23" where :1.23 currently owns org.freedesktop.Notifications?
(In reply to Simon McVittie from comment #1) > Are these messages actually sent to the destination > "org.freedesktop.Notifications", or are they in fact sent to something like > ":1.23" where :1.23 currently owns org.freedesktop.Notifications? In this particular case they were actually sent to the unique name, but the same problems occurs when I try to match on the unique name, and also when I send a message to the well-known name directly. However, it was my understanding that matching a well-known name matches messages to its owner as well. It definitely does with AddMatch().
Created attachment 119441 [details] [review] monitor: use the addressed_recipient to select matches This means we respect the destination keyword in arguments to BecomeMonitor. In bus_dispatch(), this means that we need to defer capturing until we have decided whether there is an addressed recipient; so instead of capturing once, we capture at each leaf of the decision tree. --- Intended for dbus-1.10. It's a little unfortunate that it introduces so many similar capture calls, but I think that's unavoidable.
ssh://people.freedesktop.org/~smcv/dbus monitor-destination-92074 http://cgit.freedesktop.org/~smcv/dbus/log/?h=monitor-destination-92074
Comment on attachment 119441 [details] [review] monitor: use the addressed_recipient to select matches Review of attachment 119441 [details] [review]: ----------------------------------------------------------------- I can't see anything obviously wrong here, but it would take me a long time to get familiar enough with this code to be entirely sure this is all correct and nothing's missing. Weak r+; seek further review.
Your patch fixes the bug and looks good to me (though I'm in a similar situation as Philip with regards to knowing the code). Thanks!
(In reply to Philip Withnall from comment #5) > Weak r+; seek further review. (In reply to Lars Uebernickel from comment #6) > Your patch fixes the bug and looks good to me (though I'm in a similar > situation as Philip with regards to knowing the code). I think the safest thing to do is to release 1.10.4 without this fix and a development release 1.11.0 with it, then backport to 1.10.x when either another maintainer can review it, or people have had more chance to test 1.11.0.
(In reply to Simon McVittie from comment #7) > I think the safest thing to do is to release 1.10.4 without this fix and a > development release 1.11.0 with it I did that. Jonny, I hear you're looking at monitors: perhaps you could double-check this?
I think this has been in master long enough to be reasonably confident about adding it to 1.10. Applied for 1.10.24. For the record, the original fix was indeed in 1.11.0.
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.