On Tue, Nov 11, 2025 at 11:01:44PM +0800, Jie Song wrote: > From: Jie Song <[email protected]> > > When starting a dummy QEMU process with virsh, monitor_init_qmp() enables > IOThread monitoring of the QMP fd by default. However, a race condition > exists during the initialization phase: the IOThread only removes the > main thread's fd watch when it reaches > qio_net_listener_set_client_func_full(), > which may be delayed under high system load. > > This creates a window between monitor_qmp_setup_handlers_bh() and > qio_net_listener_set_client_func_full() where both the main thread and > IOThread are simultaneously monitoring the same fd and processing events. > This race can cause either the main thread or the IOThread to hang and > become unresponsive. > > Fix this by proactively cleaning up the listener's IO sources in > monitor_init_qmp() before the IOThread initializes QMP monitoring, > ensuring exclusive fd ownership and eliminating the race condition. > > The fix introduces socket_chr_listener_cleanup() to destroy and unref > all existing IO sources on the socket chardev listener, guaranteeing > that no concurrent fd monitoring occurs during the transition to > IOThread handling. > > Signed-off-by: Jie Song <[email protected]> > --- > chardev/char-socket.c | 18 ++++++++++++++++++ > include/chardev/char-socket.h | 2 ++ > monitor/qmp.c | 6 ++++++ > 3 files changed, 26 insertions(+) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index 62852e3caf..073a9da855 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -656,6 +656,24 @@ static void tcp_chr_telnet_destroy(SocketChardev *s) > } > } > > +void socket_chr_listener_cleanup(Chardev *chr) > +{ > + SocketChardev *s = SOCKET_CHARDEV(chr); > + > + if (s->listener) { > + QIONetListener *listener = s->listener; > + size_t i; > + > + for (i = 0; i < listener->nsioc; i++) { > + if (listener->io_source[i]) { > + g_source_destroy(listener->io_source[i]); > + g_source_unref(listener->io_source[i]); > + listener->io_source[i] = NULL; > + } > + } > + } > +} > + > static void tcp_chr_update_read_handler(Chardev *chr) > { > SocketChardev *s = SOCKET_CHARDEV(chr); > diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h > index d6d13ad37f..682440c6de 100644 > --- a/include/chardev/char-socket.h > +++ b/include/chardev/char-socket.h > @@ -84,4 +84,6 @@ typedef struct SocketChardev SocketChardev; > DECLARE_INSTANCE_CHECKER(SocketChardev, SOCKET_CHARDEV, > TYPE_CHARDEV_SOCKET) > > +void socket_chr_listener_cleanup(Chardev *chr); > + > #endif /* CHAR_SOCKET_H */ > diff --git a/monitor/qmp.c b/monitor/qmp.c > index cb99a12d94..d9d1fafa70 100644 > --- a/monitor/qmp.c > +++ b/monitor/qmp.c > @@ -25,6 +25,7 @@ > #include "qemu/osdep.h" > > #include "chardev/char-io.h" > +#include "chardev/char-socket.h" > #include "monitor-internal.h" > #include "qapi/error.h" > #include "qapi/qapi-commands-control.h" > @@ -537,6 +538,11 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error > **errp) > * e.g. the chardev is in client mode, with wait=on. > */ > remove_fd_in_watch(chr); > + /* > + * Clean up listener IO sources early to prevent racy fd > + * handling between the main thread and the I/O thread. > + */ > + socket_chr_listener_cleanup(chr);
This is unsafe (may crash) because the chardev used by the monitor may not be a SocketChardev. Having todo back QMP is already calling 'remove_fd_in_watch' to purge the I/O sources. So if there is a flaw, I would expect any fix to be entirely in the chardev code, in a path from remove_fd_in_watch. > /* > * We can't call qemu_chr_fe_set_handlers() directly here > * since chardev might be running in the monitor I/O With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
