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

[GStreamer (GNOME Bugzilla)]

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

Olivier Crête <olivier.crete at ocrete.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |olivier.crete at ocrete.ca

--- Comment #5 from Olivier Crête <olivier.crete at ocrete.ca> ---
(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]:
> > 
> > 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.


> > @@ +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.

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