All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [PULL 08/27] aio: free AioContext when aio_context_new() fails
Date: Tue,  4 Nov 2025 18:53:56 +0100	[thread overview]
Message-ID: <20251104175415.525388-9-kwolf@redhat.com> (raw)
In-Reply-To: <20251104175415.525388-1-kwolf@redhat.com>

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



  parent reply	other threads:[~2025-11-04 17:58 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-04 17:53 [PULL 00/27] Block layer patches Kevin Wolf
2025-11-04 17:53 ` [PULL 01/27] aio-posix: fix race between io_uring CQE and AioHandler deletion Kevin Wolf
2025-11-04 17:53 ` [PULL 02/27] aio-posix: fix fdmon-io_uring.c timeout stack variable lifetime Kevin Wolf
2025-11-04 17:53 ` [PULL 03/27] aio-posix: fix spurious return from ->wait() due to signals Kevin Wolf
2025-11-04 17:53 ` [PULL 04/27] aio-posix: keep polling enabled with fdmon-io_uring.c Kevin Wolf
2025-11-04 17:53 ` [PULL 05/27] tests/unit: skip test-nested-aio-poll with io_uring Kevin Wolf
2025-11-04 17:53 ` [PULL 06/27] aio-posix: integrate fdmon into glib event loop Kevin Wolf
2025-11-05 15:06   ` Richard Henderson
2025-11-11 21:40     ` Kevin Wolf
2025-11-04 17:53 ` [PULL 07/27] aio: remove aio_context_use_g_source() Kevin Wolf
2025-11-04 17:53 ` Kevin Wolf [this message]
2025-11-04 17:53 ` [PULL 09/27] aio: add errp argument to aio_context_setup() Kevin Wolf
2025-11-04 17:53 ` [PULL 10/27] aio-posix: gracefully handle io_uring_queue_init() failure Kevin Wolf
2025-11-04 17:53 ` [PULL 11/27] aio-posix: unindent fdmon_io_uring_destroy() Kevin Wolf
2025-11-04 17:54 ` [PULL 12/27] aio-posix: add fdmon_ops->dispatch() Kevin Wolf
2025-11-04 17:54 ` [PULL 13/27] aio-posix: add aio_add_sqe() API for user-defined io_uring requests Kevin Wolf
2025-11-04 17:54 ` [PULL 14/27] block/io_uring: use aio_add_sqe() Kevin Wolf
2025-11-04 17:54 ` [PULL 15/27] block/io_uring: use non-vectored read/write when possible Kevin Wolf
2025-11-04 17:54 ` [PULL 16/27] block: replace TABs with space Kevin Wolf
2025-11-04 17:54 ` [PULL 17/27] block: Drop detach_subchain for bdrv_replace_node Kevin Wolf
2025-11-04 17:54 ` [PULL 18/27] iotests: Test resizing file node under raw with size/offset Kevin Wolf
2025-11-04 17:54 ` [PULL 19/27] qemu-img: Fix amend option parse error handling Kevin Wolf
2025-11-04 17:54 ` [PULL 20/27] iotests: Run iotests with sanitizers Kevin Wolf
2025-11-04 17:54 ` [PULL 21/27] qcow2: rename update_refcount_discard to queue_discard Kevin Wolf
2025-11-04 17:54 ` [PULL 22/27] qcow2: put discards in discard queue when discard-no-unref is enabled Kevin Wolf
2025-11-04 17:54 ` [PULL 23/27] tests/qemu-iotests/184: Fix skip message for qemu-img without throttle Kevin Wolf
2025-11-04 17:54 ` [PULL 24/27] tests/qemu-iotests: Improve the dry run list to speed up thorough testing Kevin Wolf
2025-11-04 17:54 ` [PULL 25/27] tests/qemu-iotest: Add more image formats to the " Kevin Wolf
2025-11-04 17:54 ` [PULL 26/27] block: Allow drivers to control protocol prefix at creation Kevin Wolf
2025-11-04 17:54 ` [PULL 27/27] qcow2, vmdk: Restrict creation with secondary file using protocol Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251104175415.525388-9-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.