[Bug 794153] GstPromise is weird

[GStreamer (GNOME Bugzilla)]

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

--- Comment #1 from Matthew Waters (ystreet00) <ystreet00 at gmail.com> ---
(In reply to Jan Alexander Steffens (heftig) from comment #0)
> I'm familiar with Promises (JavaScript async), Futures (Python concurrent
> and asyncio) and GTask. An unordered list of gripes with GstPromise:
> 
> - It has no concept of failure.

Failure us a reply with a different valu.

> - Nullable GstStructure seems to be a bad fit for a reply.

Means the value of the reply doesn't matter.  Only the act of replying has any
significance.

>   - Forced into the schema of a name + string-GValue-map.

Nothing forces you to have strings for values.

>   - Overkill for returning a simple value.

That it may be.

>   - No obvious way of signaling an error.

Return a different value.

> - No means of signaling the promiser from the promisee.

What do you mean with this?  The consumer can signal INTERRUPTED for saying it
doesn't want the producer (promiser) to produce the result anymore.

>   - No cancellation of operations other than the promiser polling the
> promise for result changes.

INTERRUPTED?

> - The purpose of INTERRUPTED and EXPIRED is not clear.

INTERRUPTED - see above.
EXPIRED - no one could answer the promise in cases where there's a message loop
in place like GstBus.

>   - INTERRUPTED results in a reply succeeding but the structure is freed.

Correct. No one cares about an interrupte promise.

>   - EXPIRED results in criticals on any manipulation, but nothing using
> GstPromise sets or checks for expiration.
>   - Was INTERRUPTED or EXPIRED meant for cancellation?
> - The promisee creating the Promise and passing it to the promiser is weird.
>   - I am used to seeing the promiser returning a promise.

The producer creating promise API has some problems with thread safety when the
producer attempts to create the promise and a change function is installed by
the consumer.

> Some suggestions:
> 
> - Split the promise into two objects created together, one for the promiser
> (GstPromise?) and one for the promisee (GstFuture?).
>   - Both sides have a change notification callback.
>   - Setting a callback on a non-PENDING side immediately fires the callback.

This also has thread safety ramifications with holding locks and such and
introduces two places for the callback to be called which we all know from pad
probe API and idle pads is a source of confusion :)

>   - The promisee side can be cancelled, then both sides become
> INTERRUPTED/CANCELLED.
>   - If any side is freed while PENDING, the other side is EXPIRED.

Relying on refs held is fraught with danger especially when the consumer is
already holding one or more refs.

>     - EXPIRED cannot be set any other way.
>     - EXPIRED on the promisee side results in a noisy warning. Promisers
> shouldn't forget about their promises!

The producer doesn't forget. It is notified no one will answer.

>   - Trying to reply to an EXPIRED or INTERRUPTED promise succeeds but does
> nothing.

This is correct for interrupted. Expired means no one is going to answer so
replying will never happen.

>   - Interruption is a signal to the promiser to abort, expiration is not
> necessarily one.

Already the case for interrupt. Expiration is the other way around and signals
that a value will never be produced to avoid pesky deadlocks waiting for a
promise.

> - Split REPLIED into RESOLVED/FULFILLED/SUCCESS/OK and
> REJECTED/FAILURE/ERROR.
>   - Success and failure come with a GValue.

This API can be built on top if you want something like this.

> - Have the promiser create the objects and return the promise.

As above, this has a problem with thread safety and deadlocks.

> - Have some additional constructors to shortcut creating a promise that's
> already resolved or already rejected.
> - Have some helper functions to shortcut fulfilling with common types or
> rejecting with a GError.

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