zbus-old icon indicating copy to clipboard operation
zbus-old copied to clipboard

`dbus_interface` macro should allow returning pre-serialized response body

Open zeenix opened this issue 5 years ago • 8 comments
trafficstars

Sometimes, you don't have ownership of the data you want to send in response to a method call so we need something like this so user don't have to clone/copy data needlessly.

@Cogitri already bumped into this issue. They were able to workaround the issue using Rc (right?) but fact remains that this should be easily possible.

I think I have a solution in mind: Add an attribute for "this method returns serialized response body":

#[dbus_interface(name = "org.myservice.Example")]
impl Example {
     #[dbus_interface(return = "serialized")]
     fn some_method(&self) -> Vec<u8> {
         ...
     }
}

The return type doesn't have to be fixed to Vec though. We could expect it to be anything AsRef<&[u8]> for example.

zeenix avatar Aug 10 '20 13:08 zeenix

In GitLab by @Cogitri on Aug 10, 2020, 15:45

They were able to workaround the issue using Rc (right?)

Nope, I switched to returning an owned struct (so I end up copying everything) because otherwise I'd have to somehow save the Rc that keeps the data alive for long enough (so I'd probably would have to save it in my DBusServer struct), but I don't want to hold it for any longer than I have to (so I want to drop it ASAP after serialization because I can't serve a new DBus query before the struct that's in the Rc has been dropped), which gets complex rather quickly

zeenix avatar Aug 10 '20 13:08 zeenix

@Cogitri Not sure I follow. You don't have to do anything extra for Rc and treat it just like a clone of the data.

zeenix avatar Aug 10 '20 14:08 zeenix

In GitLab by @Cogitri on Aug 10, 2020, 16:42

My code is something like this:

struct Database;

struct Package<'a> {
    &'a c_struct: apk_package,
}

impl Database {
    [...] // some more methods like new()
    // returned Package only lives for as long as the Database lives, after that the reference to the c_struct in Package becomes invalid
    pub fn list_available_packages(&self) -> Package {
        [...]
    }
}

struct DBusServer;

#[dbus_interface(name = "dev.Cogitri.apkPolkit1")]
impl DBusServer {
    fn list_available_packages(&self) -> fdo::Result<Vec<ApkPackage>> {
        let db = Database::new();
        let pkgs = db.list_available_packages()?;
        Ok(pkgs) // ERROR: Can't return pkgs here, since db will be dropped here and pkgs only lives for as long as db does.
    }
}

Wrapping Database in an Rc won't help here since it'll be dropped at the end of the function either way, so I'd have to keep the Database alive until serialization is done and nothing will access the ApkPackage any more. Not sure how I'd do that.

My code is probably a bit of a special case due to the constraints of the C library I'm using though :)

zeenix avatar Aug 10 '20 14:08 zeenix

Wrapping Database in an Rc won't help here since it'll be dropped at the end of the function either way,

but you clone() it, it'll be just like cloning the underlying data from Rust's POV, except that there will be no actually copying taking place.

zeenix avatar Aug 10 '20 20:08 zeenix

In GitLab by @Cogitri on Aug 10, 2020, 22:18

Yes, but I'd have to save that cloned Rc somewhere then, otherwise both the clone and the original variable will be dropped at the end of the function. It'd be hard to have the proper livetime for my Database if I save an Rc to it in my DBus struct/TLS because I'd have to know when serialisation is done (and I can drop the Package) to drop the Database.

zeenix avatar Aug 10 '20 20:08 zeenix

What lifetime? Actually you don't even need Rc.


// Database and Package remains the same

// Notice it's plural
struct Packages {
   db: Database<apk_package>,
}

impl serde::ser::Serialize for Packages {
   fn serialize(&self) -> Whatever {
         let pkgs = db.list_available_packages()?;

         pkgs.serialize()
   }
}

impl Database {
...
}

struct DBusServer;

#[dbus_interface(name = "dev.Cogitri.apkPolkit1")]
impl DBusServer {
    fn list_available_packages(&self) -> fdo::Result<Package> {
        let pkgs = Packages::new(let db = Database::new());
   
        Ok(pkgs)
    }
}

zeenix avatar Aug 10 '20 20:08 zeenix

In GitLab by @Cogitri on Aug 10, 2020, 22:35

🤯 saving the Database in the Packages struct sure is clever, how did I not think about that :D Great, thanks :)

zeenix avatar Aug 10 '20 20:08 zeenix

Np. :)

zeenix avatar Aug 10 '20 20:08 zeenix