aio: free AioContext when aio_context_new() fails
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>
This commit is contained in:
parent
d1f42b600a
commit
3769b9abe9
2 changed files with 31 additions and 3 deletions
|
|
@ -291,6 +291,9 @@ struct AioContext {
|
|||
void *epollfd_tag;
|
||||
|
||||
const FDMonOps *fdmon_ops;
|
||||
|
||||
/* Was aio_context_new() successful? */
|
||||
bool initialized;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
31
util/async.c
31
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;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue