Re: [RFC] monitor: Fix deadlock in monitor_cleanup

[Kevin Wolf <kwolf@redhat.com>]

Am 27.03.2026 um 08:56 hat hongmainquan geschrieben:
> 在 2026/3/27 03:32, Kevin Wolf 写道:
> > Am 23.03.2026 um 14:37 hat hongmianquan geschrieben:
> >> During qemu_cleanup, if a non-coroutine QMP command (e.g.,
> >> query-commands) is concurrently received and processed by the
> >> mon_iothread, it can lead to a deadlock in monitor_cleanup.
> >>
> >> The root cause is a race condition between the main thread's shutdown
> >> sequence and the coroutine's dispatching mechanism. When handling a
> >> non-coroutine QMP command, qmp_dispatcher_co schedules the actual
> >> command execution as a bottom half in iohandler_ctx and then yields.
> >> At this suspended point, qmp_dispatcher_co_busy remains true.
> >> Subsequently, the main thread in monitor_cleanup(), sets
> >> qmp_dispatcher_co_shutdown, and calls qmp_dispatcher_co_wake(). Since
> >> qmp_dispatcher_co_busy is already true, the aio_co_wake is skipped.
> >> The main thread then enters the AIO_WAIT_WHILE_UNLOCKED loop, it
> >> executes the scheduled BH (do_qmp_dispatch_bh) via
> >> aio_poll(iohandler_ctx, false), which attempts to wake up the
> >> coroutine, aio_co_wake schedules a new wake-up BH in iohandler_ctx.
> >> The main thread then blocks indefinitely in aio_poll(qemu_aio_context,
> >> true), while the coroutine's wake-up BH is starved in iohandler_ctx,
> >> qmp_dispatcher_co never reaches termination, resulting in a deadlock.
> > 
> > If the real problem is that the aio_poll() in the main thread is never
> > woken up, does this fix the problem? (Completely untested, and would
> > need a comment if we commit it.)
> > 
> After our reproduction tests, this change also resolves the issue and 
> proves to be a more appropriate solution.
> Should we proceed with this commit based on our discussion, or do you 
> require further validation?

If it works for you, let's move forward with this. I think we just
should add a comment like "if we're in AIO_WAIT_WHILE_UNLOCKED() in
monitor_cleanup(), make sure that it doesn't wait for an event in the
main context, but iohandler_ctx is polled" in the real commit because
it's not obvious why the aio_wait_kick() is needed here.

> Thanks,
> > diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> > index 9bb1e6a9f4a..0a0e9e07756 100644
> > --- a/qapi/qmp-dispatch.c
> > +++ b/qapi/qmp-dispatch.c
> > @@ -128,6 +128,7 @@ static void do_qmp_dispatch_bh(void *opaque)
> >       data->cmd->fn(data->args, data->ret, data->errp);
> >       monitor_set_cur(qemu_coroutine_self(), NULL);
> >       aio_co_wake(data->co);
> > +    aio_wait_kick();
> >   }

Do you want to send a v2 patch that takes this approach, or would you
prefer if I send it as a proper patch?

Kevin