From: Stefan Hajnoczi <[email protected]> g_source_destroy() only removes the GSource from the GMainContext it's attached to, if any. It does not free it.
Use g_source_unref() instead so that the AioContext (which embeds a GSource) is freed. There is no need to call g_source_destroy() in aio_context_new() because the GSource isn't attached to a GMainContext yet. aio_ctx_finalize() expects everything to be set up already, so introduce the new ctx->initialized boolean and do nothing when called with !initialized. This also requires moving aio_context_setup() down after event_notifier_init() since aio_ctx_finalize() won't release any resources that aio_context_setup() acquired. Signed-off-by: Stefan Hajnoczi <[email protected]> Reviewed-by: Eric Blake <[email protected]> Message-ID: <[email protected]> Reviewed-by: Kevin Wolf <[email protected]> Signed-off-by: Kevin Wolf <[email protected]> --- include/block/aio.h | 3 +++ util/async.c | 31 ++++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/include/block/aio.h b/include/block/aio.h index 3ee6015424..6cddf4d13b 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -291,6 +291,9 @@ struct AioContext { void *epollfd_tag; const FDMonOps *fdmon_ops; + + /* Was aio_context_new() successful? */ + bool initialized; }; /** diff --git a/util/async.c b/util/async.c index 9d6f0c73ee..95b1c6b610 100644 --- a/util/async.c +++ b/util/async.c @@ -366,12 +366,16 @@ aio_ctx_dispatch(GSource *source, } static void -aio_ctx_finalize(GSource *source) +aio_ctx_finalize(GSource *source) { AioContext *ctx = (AioContext *) source; QEMUBH *bh; unsigned flags; + if (!ctx->initialized) { + return; + } + thread_pool_free_aio(ctx->thread_pool); #ifdef CONFIG_LINUX_AIO @@ -580,16 +584,35 @@ AioContext *aio_context_new(Error **errp) int ret; AioContext *ctx; + /* + * ctx is freed by g_source_unref() (e.g. aio_context_unref()). ctx's + * resources are freed as follows: + * + * 1. By aio_ctx_finalize() after aio_context_new() has returned and set + * ->initialized = true. + * + * 2. By manual cleanup code in this function's error paths before goto + * fail. + * + * Be careful to free resources in both cases! + */ ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext)); QSLIST_INIT(&ctx->bh_list); QSIMPLEQ_INIT(&ctx->bh_slice_list); - aio_context_setup(ctx); ret = event_notifier_init(&ctx->notifier, false); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to initialize event notifier"); goto fail; } + + /* + * Resources cannot easily be freed manually after aio_context_setup(). If + * you add any new resources to AioContext, it's probably best to acquire + * them before aio_context_setup(). + */ + aio_context_setup(ctx); + g_source_set_can_recurse(&ctx->source, true); qemu_lockcnt_init(&ctx->list_lock); @@ -623,9 +646,11 @@ AioContext *aio_context_new(Error **errp) register_aiocontext(ctx); + ctx->initialized = true; + return ctx; fail: - g_source_destroy(&ctx->source); + g_source_unref(&ctx->source); return NULL; } -- 2.51.1
