Re: [Qemu-devel] [PATCH v1 RFC 25/34] io: add QIOTask class for async operations

[Paolo Bonzini <pbonzini@redhat.com>]

On 17/04/2015 18:11, Daniel P. Berrange wrote:
> 
> +    task = qio_task_new(OBJECT(ioc),
> +                        func, opaque, destroy);
> +
> +    qio_channel_tls_handshake_task(ioc, task);
> +
> +    object_unref(OBJECT(task));
> 
> The second ref taken at time of qio_channel_add_watch_full() gets released
> by the GDestroyNotify that is passed to that method.

/me is blind.

So this means you really do not need the reference counting;
qio_channel_tls_handshake_task (which is static anyway) is what manages
the lifetime of the QIOTask, it can take ownership of the task right
after it is created.

In effect QIOTask is just wrapping the (func, opaque, destroy)
continuation.  It makes sense that it doesn't need reference counting,
because it has a well-defined point of death (just before the
continuation is called, or just before you find out it will never be
called).  So I think it's okay to remove the reference counting and make
it a simple heap-allocated struct.  gio_channel_tls_handshake_task can
free it after calling gio_task_{abort,complete}.

Without the reference counting the GDestroyNotify also becomes
unnecessary (and the fact that you're using QIOTask as a simple
continuation explains why you didn't need GDestroyNotify), but it's okay
if you want to leave it in.

I would also have QIOTaskFunc take the "exploded" struct, i.e. typedef
void QIOTaskFunc(QMyObject *, gpointer, Error *), and hide QIOTask
entirely from the user.

Paolo