dbus2 / zbus-old

Rust D-Bus crate.
https://gitlab.freedesktop.org/dbus/zbus
Other
49 stars 13 forks source link

Method calls on server implementations where clients set NO_REPLY_EXPECTED #285

Closed zeenix closed 1 year ago

zeenix commented 2 years ago

In GitLab by @gerf on Aug 19, 2022, 02:51

I've read through the macro generation code a few times now and it seems that even though a client can set and zbus can detect NO_REPLY_EXPECTED, the macro-gen code will always expect a compatible zbus::fdo::Result is returned.

For example, if I use the following server interface code:

async fn test_write(
        &self,
        #[zbus(header)] hdr: MessageHeader<'_>,
        #[zbus(signal_context)] _ctxt: SignalContext<'_>,
        #[zbus(object_server)] _server: &ObjectServer,
        arg: &str,
    ) {
        if hdr
            .primary()
            .flags()
            .contains(zbus::MessageFlags::NoReplyExpected)
        {
            debug!("asked not to reply, but server does not yet support ignoring");
        }
    }
}

I can detect that the client has specified this header, but can't actually stop a reply from being generated. Typically this generates a method_return reply. I could, of course, use the lower-level API's to implement this method call, but that's a lot more work than using the macro interface.

It might be nice to have an additional macro for the server interface that allows control of sending replies or some other kind of type that indicates a reply should not be sent, like Result::Ignore or something. Also quite possible that I have mis-understood this codebase and what I want to accomplish is already possible.

zeenix commented 2 years ago

Right, this support was already added for dbus_proxy (!409) but not dbus_interface. I'd be very happy to land this support. If you can provide a merge request, that'd be super helpful. The commits in !409 should give you a good idea of how we want to handle this.

zeenix commented 2 years ago

In GitLab by @gerf on Aug 19, 2022, 20:30

Thank you for the pointer to implementation hints. To clarify, you would prefer to treat this as a macro attribute and if set, no reply would be generated by the method?

For example:

#[dbus_interface(name = "io.fake.SomeServer")]
impl SomeServer {

#[dbus_interface(no_reply)]
async fn test_write(&mut self, data: &str) { self.data = data; }
}

If that's the case, the issue that I see with this approach is since the client can set the flag, supporting this at compile time would permanently make this method return nothing when there are potential use-cases to return nothing and something depending on the circumstances. I believe the client will hang or time out if the no reply flag is not set and the server doesn't respond. The DBus docs say:

If a METHOD_CALL message has the flag NO_REPLY_EXPECTED, then the application receiving the method should not send the reply message (regardless of whether the reply would have been METHOD_RETURN or ERROR).

I'm not sure how to reconcile the client setting this flag other than forced checks at runtime, but I could be missing something about the specification. I would think introspection would tell you a server method doesn't return. I believe the client proxy xml has annotations for this as well.

zeenix commented 2 years ago

I believe I sent you in the wrong direction, sorry. :( You're right, this is not something that needs any macro work. This needs to be handled by the ObjectServer itself in it's dispatch handler. It just need to skip sending of any reply from the method.

zeenix commented 2 years ago

In GitLab by @gerf on Aug 22, 2022, 22:21

Thank you for pointing me in the right direction. I have opened a merge request here: https://gitlab.freedesktop.org/dbus/zbus/-/merge_requests/553 for this addition.