From: Stefan Hajnoczi <stefanha@redhat.com>
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 <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-ID: <20251104022933.618123-9-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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 1657740a0e..2760f308f5 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -291,6 +291,9 @@ struct AioContext {
gpointer epollfd_tag;
const FDMonOps *fdmon_ops;
+
+ /* Was aio_context_new() successful? */
+ bool initialized;
};
/**
diff --git a/util/async.c b/util/async.c
index cb72ad3777..7d06ff98f3 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
@@ -579,16 +583,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);
@@ -622,9 +645,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