Timeout on ObjectServer #211

Closed zeenix closed 1 year ago

In GitLab by @elmarco on Sep 8, 2021, 22:02

Typically, a DBus service should exit after a period of inactivity.

In sync API, I think such API could look like

loop {
    if connection.object_server().wait(Duration::new(60, 0)) {
        // timeout, break/exit()
    }
}

Look ok? Any idea how to do that?

Typically, a DBus service should exit after a period of inactivity.

True but as pointed out on the Matrix, the timeout is for inactivity in general and D-Bus inactivity is just a part of it. It's also not clear what counts as "D-Bus activity" and what doesn't. E.g Does emitting signals count or only incoming method calls? Do the fdo service activity (as you pointed out) that we implicitly implement count or not?

Also, do other dbus libraries provide such facilities? Not saying that's a good reason not to have it (we want to be the best, after all) but if they don't, I think the reasons outlined in the previous para might be why.

I'd also rather not have the user keep a read lock on the ObjectServer for long time. At the moment, the method dispatch works with read lock so there is no problem implementing your suggestion but if we switch the dispatch to write lock for any reason, we'll be in trouble. Hence the suggestion of using event_listener (or channel) on Matrix, if we do provide a direct way to wait for inactivity timeout.

So for now at least, I think we should leave this to the service impl. but rather recommend a few different ways in the docs. But if you can come up with some nice and robust API, I'll be more than happy.

In GitLab by @danieldg on Sep 11, 2021, 20:09

Can't you do this right now using code like this?

loop {
   match select(timeout(duration), message_stream.filter(/* method calls */).next()).await {
      Either::Left(_) => break,
      _ => continue,
   }
}

In GitLab by @elmarco on Sep 11, 2021, 20:22

Can't you do this right now using code like this?

That doesn't work for sync code, or interface servicing

In GitLab by @danieldg on Sep 11, 2021, 20:29

It's not that hard to write a sync version, though you would need to spawn a dedicated helper thread because zbus doesn't have any cancellation or wait-timeout in its sync API.

What is "interface servicing"? Any calls on any dbus interface will avoid the timeout, that's the whole point.

In GitLab by @elmarco on Sep 11, 2021, 20:32

I am afraid I don't follow you. Maybe you could write an example or a patch?

In GitLab by @danieldg on Sep 11, 2021, 20:36

let (send, recv) = std::sync::mpsc::channel();
let activity = Thread::new(move || {
   for message in message_stream {
      if /* message is method call */ {
         send.send(());
      }
   }
});
loop {
   match recv.recv_timeout(duration) {
      Err(std::sync::mpsc::RecvTimeoutError::Timeout) => break,
      _ => continue,
   }
}

In GitLab by @elmarco on Sep 11, 2021, 20:39

So you are suggesting zbus code changes, right? Have you seen the related MR !378. It's simpler.

In GitLab by @danieldg on Sep 11, 2021, 20:39

No, this is not a zbus code change. This is client code.

In GitLab by @elmarco on Sep 11, 2021, 20:40

How does this integrate with ObjectServer & interface dispatching ?

In GitLab by @danieldg on Sep 11, 2021, 20:42

It gets the messages at the same time as objectserver, so any messages coming in to the objectserver will reset the timeout.

In GitLab by @elmarco on Sep 11, 2021, 20:47

I see, that could work for incoming activity. Arguably the outgoing activity can be monitored by the user itself and synthesize a global timeout. But the proposed MR makes such monitoring easier for the user to implement connection timeout though.

In GitLab by @danieldg on Sep 11, 2021, 20:54

A connection timeout is usually about "not seen incoming requests for N" not "have not sent anything in N" - and you're right, the MR makes the latter easier to implement.

In GitLab by @elmarco on Sep 21, 2021, 15:21

mentioned in commit ebbafce1b9e6a6684b6f247be43e46e0f30ffa6b

dbus2 / zbus-old

Timeout on ObjectServer #211