Re: API update proposal: cancellation notification

[Iago Toral <itoral igalia com>]

Hi,

On Mon, 24 Jan 2011 14:28:46 +0100, Guillaume Emont <gemont igalia com> 
wrote:
> Hi people!
>
>  We've discussed a bit on IRC that it might be useful for the caller
> to be notified once an operation has been cancelled.
>  The rationale is that cancellation is an asynchronous operation,
> that will not be finished (the cancelled operation won't be
> terminated) immediately.
>
>  So, a "quick" poll
>
>  1/ Do you agree this is needed?

Well, I would not say that it is "needed", I would say it is 
convenient, at least in some situations.

>  2/ On IRC, and in discussions with Juan, we thought of a few ways to
> implement it, which one do you prefer? Do you have a better idea? Do
> you see more pros/cons to some of these ideas
>
>  a/ Call the GrlMediaSourceResultCb callback with a special value to
> the remaining parameter (GRL_SOURCE_OP_CANCELLED), that would 
> probably
> be implemented as (guint)-2 (aka G_MAXUINT-1).
>  Pros:
>  - not too intrusive, small enough change for all parts (core,
> plugins, caller)
>  Cons:
>  - makes the semantic of remaining ambivalent
>  - won't work for metadata callbacks that don't have a remaining
> parameter

I guess this second "con" is big enough to discard this idea.
However, we could have a new parameter (a boolean maybe) to give this 
information.

>  b/ Call the operation callback with a GError set to
> GRL_CORE_ERROR_OPERATION_CANCELLED.
>  Pros:
>  - consistent with GIO's GCancellable stuff, such as
> g_file_read_async()
>  - not too intrusive, small changes. If the caller does not care
> about it, it might even be able to treat it with the same code as for
> handling other errors
>  - works for all operations
>  Cons:
>  - Cancellation might not be an error stricto sensu, though others
> (GIO at least) do it that way.

Personally, I found it semantically incorrect to use an error 
notification for a cancelled operation when cancellation is not the 
consequence of an error. Still, if some other projects are using this 
convention I guess that's something I can live with.

>  c/ Provide a cancelled signal that takes a callback like
> cancelled_cb (GrlMediaSource *source, guint operation_id, gpointer
> user_data) (or the same with a GrlMetadataSource)
>  Pros:
>  - will not change existing API at all, only a new signal added
>  Cons:
>  - For MediaSource, does not call the callback one last time, like
> when there is no more element
>  - Might be tricky to ensure we avoid race conditions, see
>
> http://library.gnome.org/devel//gio/2.27/GCancellable.html#GCancellable-cancelled
> [1]

I don't like this very much. I want to know that I only have to put my 
"operation finished" code in one place.

>  d/ Take a GCancellable as parameter for the operation calls. These
> already provide a complete API, as well as a "cancelled" signal. For
> consistency, the _cancel() call would be deprecated. If implementing
> this solution, it would be consistent to implement (b) as well.
>  Pros:
>  - already using an existing framework we wouldn't have to debug, and
> some programmers would already know it.
>  Cons:
>  - significant API change, changing the way a cancellation is done,
> and adding a parameter to operations.

There is another obvious benefit: the cancellation code in the core of 
grilo is a bit tricky (not because it does weird things or anything, but 
becuse it is complex to change and not break something). Using 
GCancellable would allow us to forget about this.

>  Note that some of these proposals could be combined, for instance,
> we could both call the operation callback with an error set (b) and
> emit a "cancelled" signal (c).
>
>  From my point of view, the nicer API would be (d) and (b)
> (consistency with glib asynchronous stuff sounds like a good idea),
> but the most practical would be (b) or (b)+(c)
>
>  Thanks for reading, and thanks for your replies.

Thinking in the long run, I say we should do b+d. We can do that 
progressively and start with b only and then do the d part which is the 
most intrusive.

btw, good summary of the options available, thanks! :)

Iago