Cache well-known names for signal streams

[zeenix]

The following discussion from !410 should be addressed:

• [ ] @zeenix started a discussion: (+11 comments)

  for a second there I got scared that this method call is invoked on each signal received. :)

  In the future it may be possible to re-enable caching of GetNameOwner, but this requires more extensive changes.

  While we don't need to do this right away, we do need to be sure this will be possible to do w/o breaking API. Naive question but why can't we just use an async lock around destination to ensure it's cached before the first stream is created?

[zeenix]

In GitLab by @danieldg on Oct 29, 2021, 01:29

A few races to avoid:

Case A (requires separate tasks for GetNameOwner tracking and SignalStream):

1. NameOwnerChanged("org.example.Foo", "", "ClientA")
2. Foo::HelloSignal

You get a race when the signal task runs before the NameOwner tracking task.

Case B (requires separate tasks for GetNameOwner tracking and SignalStream):

1. Foo::ProcessingFinishedSignal("successful, foo exiting", "/opt/foo/outputs/ABCD1234")
2. NameOwnerChanged("org.example.Foo", "ClientA", "")

You get a race when the signal task runs after the NameOwner tracking task.

Case C (GetNameOwner tracking is part of SignalStream, but is bootstrapped using a separate NameOwnerChanged tracking task):

1. DesktopLauncher::StartupNotification("org.example.Foo")
2. NameOwnerChanged("org.example.Foo", "", "ClientA")
3. (later) Foo::SomeNormalSignal

User code: a StartupNotification event triggers the creation of a new task with a new Proxy based on the name passed in the signal.

Race: If the creation of the SignalStream's Receiver happens after the broadcast of (2), but before the NameOwnerChanged task has processed message (2), then the stream is initialized with the fact "Foo has no owner", and will never receive any signals until the ownership of Foo changes hands.

While it's reasonable for this user code to miss a few early signals from Foo, the user should be able to rely on getting any signals that happen after a method they call on Foo from their new task. The issue is that they never get any Foo signals because the stream is still waiting to get notified of take-ownership.

[zeenix]

Case C (GetNameOwner tracking is part of SignalStream, but is bootstrapped using a separate NameOwnerChanged tracking task):

1. DesktopLauncher::StartupNotification("org.example.Foo")
2. NameOwnerChanged("org.example.Foo", "", "ClientA")
3. (later) Foo::SomeNormalSignal

User code: a StartupNotification event triggers the creation of a new task with a new Proxy based on the name passed in the signal.

Tbh that seems like a bad D-Bus API design, if there is no API provided for clients to catch up with the state of the service after the service has already launched. At the very least, the service should first register its name and then announce the newly available name.

[zeenix]

In GitLab by @danieldg on Oct 29, 2021, 16:57

• The DesktopLauncher signal is sent by a different client than ClientA.
• I did assume there is a catch-up query - that's what the "it's reasonable for this user code to miss a few early signals from Foo" statement is about. The issue is that once the catch-up query is made, you still don't get any signals from the stream.

A more complete race:

1. DesktopLauncher::StartupNotification("org.example.Foo")
2. NameOwnerChanged("ClientA", "", "ClientA") (a new dbus client)
3. NameOwnerChanged("org.example.Foo", "", "ClientA")
4. Foo::HelloSignal
5. (sent by the zbus client) Foo::GetBarState
6. Foo::GetBarState successful reply
7. Foo::BarStateChangedSignal

The zbus client would also need to handle the Foo::GetBarState returning an error (no such endpoint), but if that happens then it knows that it will get the HelloSignal when ClientA finally shows up.