On Wed, Jan 7, 2026 at 7:36 PM Daniel P. Berrangé <[email protected]> wrote: > > The original design of QIOTask was intended to simplify lifecycle > management by automatically freeing it when the task was marked as > complete. This overlooked the fact that when a QIOTask is used in > combination with a GSource, there may be times when the source > callback is never invoked. This is typically when a GSource is > released before any I/O event arrives. In such cases it is not > desirable to mark a QIOTask as complete, but it still needs to be > freed. To satisfy this, the task must be released manually. > > Signed-off-by: Daniel P. Berrangé <[email protected]>
Reviewed-by: Marc-André Lureau <[email protected]> > --- > include/io/task.h | 29 +++++++++++++++++++++-------- > io/channel-tls.c | 4 ++++ > io/channel-websock.c | 3 +++ > io/task.c | 8 ++++++-- > tests/unit/test-io-task.c | 26 ++++++++++++++++++++++++++ > 5 files changed, 60 insertions(+), 10 deletions(-) > > diff --git a/include/io/task.h b/include/io/task.h > index 0b5342ee84..98847f5994 100644 > --- a/include/io/task.h > +++ b/include/io/task.h > @@ -96,7 +96,7 @@ typedef void (*QIOTaskWorker)(QIOTask *task, > * 1000, > * myobject_operation_timer, > * task, > - * NULL); > + * qio_task_free); > * } > * </programlisting> > * </example> > @@ -138,9 +138,8 @@ typedef void (*QIOTaskWorker)(QIOTask *task, > * the callback func 'myobject_operation_notify' shown > * earlier to deal with the results. > * > - * Once this function returns false, object_unref will be called > - * automatically on the task causing it to be released and the > - * ref on QMyObject dropped too. > + * Once this function returns FALSE, the task will be freed, > + * causing it release the ref on QMyObject too. > * > * The QIOTask module can also be used to perform operations > * in a background thread context, while still reporting the > @@ -208,8 +207,8 @@ typedef void (*QIOTaskWorker)(QIOTask *task, > * 'err' attribute in the task object to determine if > * the operation was successful or not. > * > - * The returned task will be released when qio_task_complete() > - * is invoked. > + * The returned task must be released by calling > + * qio_task_free() when no longer required. > * > * Returns: the task struct > */ > @@ -218,6 +217,19 @@ QIOTask *qio_task_new(Object *source, > gpointer opaque, > GDestroyNotify destroy); > > +/** > + * qio_task_free: > + * task: the task object to free > + * > + * Free the resources associated with the task. Typically > + * the qio_task_complete() method will be called immediately > + * before this to trigger the task callback, however, it is > + * permissible to free the task in the case of cancellation. > + * The destroy callback will be used to release the opaque > + * data provided to qio_task_new(). > + */ > +void qio_task_free(QIOTask *task); > + > /** > * qio_task_run_in_thread: > * @task: the task struct > @@ -268,8 +280,9 @@ void qio_task_wait_thread(QIOTask *task); > * qio_task_complete: > * @task: the task struct > * > - * Invoke the completion callback for @task and > - * then free its memory. > + * Invoke the completion callback for @task. This should typically > + * only be invoked once on a task, and then qio_task_free() used > + * to free it. > */ > void qio_task_complete(QIOTask *task); > > diff --git a/io/channel-tls.c b/io/channel-tls.c > index b0cec27cb9..07274c12df 100644 > --- a/io/channel-tls.c > +++ b/io/channel-tls.c > @@ -170,6 +170,7 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS > *ioc, > trace_qio_channel_tls_handshake_fail(ioc); > qio_task_set_error(task, err); > qio_task_complete(task); > + qio_task_free(task); > return; > } > > @@ -183,6 +184,7 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS > *ioc, > trace_qio_channel_tls_credentials_allow(ioc); > } > qio_task_complete(task); > + qio_task_free(task); > } else { > GIOCondition condition; > QIOChannelTLSData *data = g_new0(typeof(*data), 1); > @@ -270,11 +272,13 @@ static void qio_channel_tls_bye_task(QIOChannelTLS > *ioc, QIOTask *task, > trace_qio_channel_tls_bye_fail(ioc); > qio_task_set_error(task, err); > qio_task_complete(task); > + qio_task_free(task); > return; > } > > if (status == QCRYPTO_TLS_BYE_COMPLETE) { > qio_task_complete(task); > + qio_task_free(task); > return; > } > > diff --git a/io/channel-websock.c b/io/channel-websock.c > index cb4dafdebb..b4f96a0af4 100644 > --- a/io/channel-websock.c > +++ b/io/channel-websock.c > @@ -545,6 +545,7 @@ static gboolean > qio_channel_websock_handshake_send(QIOChannel *ioc, > trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err)); > qio_task_set_error(task, err); > qio_task_complete(task); > + qio_task_free(task); > wioc->hs_io_tag = 0; > return FALSE; > } > @@ -561,6 +562,7 @@ static gboolean > qio_channel_websock_handshake_send(QIOChannel *ioc, > trace_qio_channel_websock_handshake_complete(ioc); > qio_task_complete(task); > } > + qio_task_free(task); > wioc->hs_io_tag = 0; > return FALSE; > } > @@ -588,6 +590,7 @@ static gboolean > qio_channel_websock_handshake_io(QIOChannel *ioc, > trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err)); > qio_task_set_error(task, err); > qio_task_complete(task); > + qio_task_free(task); > wioc->hs_io_tag = 0; > return FALSE; > } > diff --git a/io/task.c b/io/task.c > index 451f26f8b4..331febd4e1 100644 > --- a/io/task.c > +++ b/io/task.c > @@ -70,8 +70,12 @@ QIOTask *qio_task_new(Object *source, > return task; > } > > -static void qio_task_free(QIOTask *task) > +void qio_task_free(QIOTask *task) > { > + if (!task) { > + return; > + } > + > qemu_mutex_lock(&task->thread_lock); > if (task->thread) { > if (task->thread->destroy) { > @@ -110,6 +114,7 @@ static gboolean qio_task_thread_result(gpointer opaque) > > trace_qio_task_thread_result(task); > qio_task_complete(task); > + qio_task_free(task); > > return FALSE; > } > @@ -196,7 +201,6 @@ void qio_task_complete(QIOTask *task) > { > task->func(task, task->opaque); > trace_qio_task_complete(task); > - qio_task_free(task); > } > > > diff --git a/tests/unit/test-io-task.c b/tests/unit/test-io-task.c > index 115dba8970..b1c8ecb7ab 100644 > --- a/tests/unit/test-io-task.c > +++ b/tests/unit/test-io-task.c > @@ -73,6 +73,7 @@ static void test_task_complete(void) > src = qio_task_get_source(task); > > qio_task_complete(task); > + qio_task_free(task); > > g_assert(obj == src); > > @@ -84,6 +85,28 @@ static void test_task_complete(void) > } > > > +static void test_task_cancel(void) > +{ > + QIOTask *task; > + Object *obj = object_new(TYPE_DUMMY); > + Object *src; > + struct TestTaskData data = { NULL, NULL, false }; > + > + task = qio_task_new(obj, task_callback, &data, NULL); > + src = qio_task_get_source(task); > + > + qio_task_free(task); > + > + g_assert(obj == src); > + > + object_unref(obj); > + > + g_assert(data.source == NULL); > + g_assert(data.err == NULL); > + g_assert(data.freed == false); > +} > + > + > static void task_data_free(gpointer opaque) > { > struct TestTaskData *data = opaque; > @@ -101,6 +124,7 @@ static void test_task_data_free(void) > task = qio_task_new(obj, task_callback, &data, task_data_free); > > qio_task_complete(task); > + qio_task_free(task); > > object_unref(obj); > > @@ -123,6 +147,7 @@ static void test_task_failure(void) > > qio_task_set_error(task, err); > qio_task_complete(task); > + qio_task_free(task); > > object_unref(obj); > > @@ -260,6 +285,7 @@ int main(int argc, char **argv) > module_call_init(MODULE_INIT_QOM); > type_register_static(&dummy_info); > g_test_add_func("/crypto/task/complete", test_task_complete); > + g_test_add_func("/crypto/task/cancel", test_task_cancel); > g_test_add_func("/crypto/task/datafree", test_task_data_free); > g_test_add_func("/crypto/task/failure", test_task_failure); > g_test_add_func("/crypto/task/thread_complete", > test_task_thread_complete); > -- > 2.52.0 > > -- Marc-André Lureau
