[Bug 789843] API: GstPromise - object with promise/future-like semantics

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=789843

--- Comment #6 from Matthew Waters (ystreet00) <ystreet00 at gmail.com> ---
(In reply to Olivier Crête from comment #5)
> (In reply to Matthew Waters (ystreet00) from comment #4)
> > (In reply to Olivier Crête from comment #2)
> > > Review of attachment 362870 [details] [review] [review] [review]:
> > > 
> > > The API seems relatively sane. The only thing I'm not so happy about is the
> > > change function being called synchronously with the mutex held, is that
> > > really required? I'd feel better having it called from another thread, doing
> > > something like gst_element_call_async(), it could even be using the same
> > > thread pool.
> > 
> > Yea, see I'm very hesitant to add another thread into the mix.
> > 
> > Removing the change_func callback outside the mutex also introduces a race
> > whereby gst_promise_set_change_callback() can race with calling the
> > change_func and access freed data.
> > 
> > i.e. before the change_func is called and after the state has changed, it's
> > possible to 1. change the function but not the data, 2. notify (and
> > therefore free) the data that will be used in the state change.  Both of
> > these are pretty severe memory issues :).  The only way I can see to
> > mitigate these is some extra checking to e.g. seal the state change function
> > once set and passed for reply and not allow that scenario to occur.
> 
> I'd be favorable to making it possible to set the callback function+data
> multiple times and then stealing it when it's triggered.. And immediately
> sending it to a thread to be triggered if it's set after the promise has
> been acted upon, to prevent the race where it's set after the promise has
> been replied/interrrupted/etc.

Originally I had the webrtcbin implementation so that the callee had to create
the promise and thus the setting of the change callback could race with the
reply.  That's since been changed so that the caller creates the promise and
sets the change callback before passing for a reply.

I don't really want to support the first case as setting the change callback
after a reply/interrupt/expire doesn't seem like a valid use case?

Stealing the func/data is a good idea though.

> > > @@ +157,3 @@
> > > +    GST_LOG ("%p replied", promise);
> > > +
> > > +    promise->promise = s;
> > > 
> > > Maybe you want to also do gst_structure_set_parent_refcount() ? Or does it
> > > make no sense at all?
> > 
> > It sort of makes sense to do that however the idea is that once the
> > structure is set on the promise, it is immutable which is not quite what
> > gst_structure_set_parent_refcount() ensures.
> 
> I wonder if we can create a "const int refcount = 2;" and then set that as
> the refcoun to make it immutable.

That sounds crazy but actually looks like it might work :)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.