Bug 38784 - Add standard DBus interface for managing the lifetime of objects based on shared ownership
Add standard DBus interface for managing the lifetime of objects based on sha...
Status: REOPENED
Product: dbus
Classification: Unclassified
Component: core
1.5
All All
: low enhancement
Assigned To: D-Bus Maintainers
D-Bus Maintainers
waiting for more users
: patch
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-29 14:34 UTC by Will Manley
Modified: 2014-09-25 14:51 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to DBus specification describing interface RefCounted (6.66 KB, patch)
2011-06-29 14:34 UTC, Will Manley
Details | Splinter Review
Patch to DBus specification describing interface RefCounted V2 (6.83 KB, patch)
2011-08-10 13:11 UTC, Will Manley
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Will Manley 2011-06-29 14:34:24 UTC
Created attachment 48571 [details] [review]
Patch to DBus specification describing interface RefCounted

In our system[1] we have a DBus server that is in charge of playing back, recording, buffering, etc. video media.  Clients that want a video stream recorded or played back ask a DBus server to create an object for doing this.  While the object lives in the server, really it belongs to the client, and as such the server should remove it from its connection and and destroy it when the client becomes disinterested in it.  There is the additional complexity that the server might have sent one of these objects to multiple clients or may have sent the same object to a particular client multiple times.

I wanted to extend the warm fuzzy feeling that in-process reference counted pointers gives you, but to our multi-process system.  I wanted it to be asynchronous and non-chatty and I think I have achieved this.

Given that it was quite tricky to get right I thought it might be useful to other DBus users for it to become a standard DBus interface.  That way it is more likely that it might be implemented in bindings (other than my own).

I have attached a patch to the DBus spec describing what I've done.  I'm not expecting it to be immediately accepted but I created the patch so we would have something concrete to discuss.  As smcv said on IRC today "my rule of thumb would be, propose it as a standard D-Bus interface when you find your second use-case ;)".  Hopefully this use-case will surface.

[1] YouView TV set top boxes ( http://www.youview.com )
Comment 1 Simon McVittie 2011-07-11 04:01:20 UTC
Review of attachment 48571 [details] [review]:

As I said, I'd rather not have this as a "standard" interface until a second potential user appears (YouView + something else), but for the future:

::: doc/dbus-specification.xml
@@ +3204,3 @@
+      </para>
+      <para>
+        <literal>DropRef</literal> will not return.

I don't like this - in principle, every D-Bus method call should return (but as an optimization, if the NO_REPLY flag is set, the service may avoid returning).

So, I'd prefer it phrased more like:

DropRef does not return a useful result, and calling it with the NO_REPLY flag is suggested.

@@ +3213,3 @@
+        every RefCounted object.  One is in the client and one is in the
+        remote application.  (In fact the remote application has a
+        per client reference count for each object).  In the discussion

It'd probably be clearer if you moved the definition of "server" to the top, then used that?

The fact that the object might be refcounted inside the server is really an implementation detail. What's important is: the server maintains a separate refcount on behalf of each client (identifying clients by their unique connection name) and keeps the object intact as long as at least one client has a reference; and each client cooperates with the server to keep its refcount up to date.

@@ +3222,3 @@
+              Every time an object is returned to a client from a remote
+              application the reference count in the server for that
+              object/client is incremented.

Perhaps:

Every time a client calls a method that sends a RefCounted object's path in its method reply message, the reference count maintained on behalf of that client is incremented.

@@ +3228,3 @@
+            <para>
+              Whenever the client receives an object path the reference count
+              in the client for that object is incremented.

The fact that the client is expected to track refs internally is just a consequence of the API; it's not really part of the API, as such.

@@ +3271,3 @@
+              Reference counts are not affected for DBus paths sent as a
+              signal payload as it is impossible to know who has received
+              these messages.

In Telepathy we have the concept of "handles", which are/were a bit like RefCounted. We've found that it's often useful for APIs to guarantee that if a "parent" object emits a signal mentioning a refcounted "child", then the "child" will exist for at least as long as the "parent" does, even if no client holds a ref.

This does need to be case-by-case, though.

@@ +3294,3 @@
+        it has been confirmed that Client B has called <literal>AddRef</literal>
+        on the object to avoid race conditions.  <literal>AddRef</literal>
+        will not return.

Again, I'd prefer to turn this into "calling it with NO_REPLY is suggested". If called without NO_REPLY, it should reply (that's part of the - mostly unwritten - D-Bus object model).

@@ +3300,3 @@
+          The <literal>org.freedesktop.DBus.RefCounted</literal>
+          interface was added in version 0.18 of the D-Bus
+          specification.

In Telepathy we have a convention of not mentioning concrete version numbers until things are at least merged, and usually also about-to-be-released - we call it "version UNRELEASED" or "version 0.UNRELEASED" or something, and grep for UNRELEASED/fix the references just before releasing.

I think that might be useful for the D-Bus Specification too, particularly for things that are sitting in bugzilla awaiting a second use-case.
Comment 2 Simon McVittie 2011-07-27 03:10:29 UTC
(The D-Bus specification is actually dbus/core, dbus/doc is something else.)
Comment 3 Will Manley 2011-08-10 13:11:55 UTC
Created attachment 50102 [details] [review]
Patch to DBus specification describing interface RefCounted V2
Comment 4 Will Manley 2011-08-10 13:25:32 UTC
Thanks for the review.  I've made some changes accordingly

(In reply to comment #1)
> Review of attachment 48571 [details] [review]:
> 
> As I said, I'd rather not have this as a "standard" interface until a second
> potential user appears (YouView + something else), but for the future:
> 
> ::: doc/dbus-specification.xml
> @@ +3204,3 @@
> +      </para>
> +      <para>
> +        <literal>DropRef</literal> will not return.
> 
> I don't like this - in principle, every D-Bus method call should return (but as
> an optimization, if the NO_REPLY flag is set, the service may avoid returning).

Sounds sensible.  Fixed.

> @@ +3213,3 @@
> +        every RefCounted object.  One is in the client and one is in the
> +        remote application.  (In fact the remote application has a
> +        per client reference count for each object).  In the discussion
> 
> It'd probably be clearer if you moved the definition of "server" to the top,
> then used that?

Quite right, I was clinging to "remote application" as I saw it in other parts of the spec.  It sounds a lot less clumsy with server defined higher.

> The fact that the object might be refcounted inside the server is really an
> implementation detail. What's important is: the server maintains a separate
> refcount on behalf of each client (identifying clients by their unique
> connection name) and keeps the object intact as long as at least one client has
> a reference; and each client cooperates with the server to keep its refcount up
> to date.

I agree that much of this section is an implementation detail, but I believe there is probably only one sensible implementation so spelling it out seemed clearer.  Originally I had used "should" all over the place but it didn't read so well.  In particular when I've presented this to others in my organisation the initial confusion was related to not understanding that there were actually *two* reference counts in the system so I thought it was important state it explicitly.

> @@ +3222,3 @@
> +              Every time an object is returned to a client from a remote
> +              application the reference count in the server for that
> +              object/client is incremented.
> 
> Perhaps:
> 
> Every time a client calls a method that sends a RefCounted object's path in its
> method reply message, the reference count maintained on behalf of that client
> is incremented.

Agreed, that is much clearer.

> 
> @@ +3228,3 @@
> +            <para>
> +              Whenever the client receives an object path the reference count
> +              in the client for that object is incremented.
> 
> The fact that the client is expected to track refs internally is just a
> consequence of the API; it's not really part of the API, as such.

I still think it is important to state for the same reasons as above.

> @@ +3271,3 @@
> +              Reference counts are not affected for DBus paths sent as a
> +              signal payload as it is impossible to know who has received
> +              these messages.
> 
> In Telepathy we have the concept of "handles", which are/were a bit like
> RefCounted. We've found that it's often useful for APIs to guarantee that if a
> "parent" object emits a signal mentioning a refcounted "child", then the
> "child" will exist for at least as long as the "parent" does, even if no client
> holds a ref.
> 
> This does need to be case-by-case, though.

Sounds interesting.  I see no reason that the fact that a RefCounted object be solely kept alive by the refcount as long as it is kept alive when the refcount is >0.  Do you think this or something like it should be explicitly called out?

> @@ +3294,3 @@
> +        it has been confirmed that Client B has called
> <literal>AddRef</literal>
> +        on the object to avoid race conditions.  <literal>AddRef</literal>
> +        will not return.
> 
> Again, I'd prefer to turn this into "calling it with NO_REPLY is suggested". If
> called without NO_REPLY, it should reply (that's part of the - mostly unwritten
> - D-Bus object model).

Yep, done

> @@ +3300,3 @@
> +          The <literal>org.freedesktop.DBus.RefCounted</literal>
> +          interface was added in version 0.18 of the D-Bus
> +          specification.
> 
> In Telepathy we have a convention of not mentioning concrete version numbers
> until things are at least merged, and usually also about-to-be-released - we
> call it "version UNRELEASED" or "version 0.UNRELEASED" or something, and grep
> for UNRELEASED/fix the references just before releasing.
> 
> I think that might be useful for the D-Bus Specification too, particularly for
> things that are sitting in bugzilla awaiting a second use-case.

Sounds sensible - done