Bug 92074

Summary: destination= rules don't match anything when using BecomeMonitor
Product: dbus Reporter: Lars Uebernickel <lars>
Component: coreAssignee: D-Bus Maintainers <dbus>
Status: RESOLVED FIXED QA Contact: D-Bus Maintainers <dbus>
Severity: normal    
Priority: medium CC: jonny.lamb
Version: 1.10Keywords: patch, regression
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: monitor: use the addressed_recipient to select matches

Description Lars Uebernickel 2015-09-22 13:37:12 UTC
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.
Comment 1 Simon McVittie 2015-09-22 14:03:26 UTC
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?
Comment 2 Lars Uebernickel 2015-09-22 14:10:49 UTC
(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().
Comment 3 Simon McVittie 2015-11-06 14:56:10 UTC
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.
Comment 4 Simon McVittie 2015-11-07 10:31:11 UTC
ssh://people.freedesktop.org/~smcv/dbus monitor-destination-92074
http://cgit.freedesktop.org/~smcv/dbus/log/?h=monitor-destination-92074
Comment 5 Philip Withnall 2015-11-07 11:03:04 UTC
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.
Comment 6 Lars Uebernickel 2015-11-17 11:38:33 UTC
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!
Comment 7 Simon McVittie 2015-11-17 15:00:53 UTC
(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.
Comment 8 Simon McVittie 2016-02-08 13:58:54 UTC
(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?
Comment 9 Simon McVittie 2017-09-25 16:08:45 UTC
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.