From: Jie Song <mail@jiesong.me>
To: eblake@redhat.com, armbru@redhat.com, berrange@redhat.com,
qemu-devel@nongnu.org
Cc: mail@jiesong.me, songjie_yewu@cmss.chinamobile.com
Subject: [PATCH v3] monitor/qmp: cleanup SocketChardev listener sources early to avoid fd handling race
Date: Sat, 22 Nov 2025 09:46:56 +0800 [thread overview]
Message-ID: <20251122014656.297693-1-mail@jiesong.me> (raw)
From: Jie Song <songjie_yewu@cmss.chinamobile.com>
When starting a dummy QEMU process with virsh version, 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.
Signed-off-by: Jie Song <songjie_yewu@cmss.chinamobile.com>
---
Changes in v3:
- Use a more general method to fix the problem.
- Link to v2:
https://lore.kernel.org/qemu-devel/20251117150142.131694-1-mail@jiesong.me/
- Link to v1:
https://lore.kernel.org/qemu-devel/20251111150144.76751-1-mail@jiesong.me/
---
chardev/char-io.c | 8 ++++++++
chardev/char-socket.c | 9 +++++++++
include/chardev/char-io.h | 2 ++
include/chardev/char.h | 2 ++
monitor/qmp.c | 5 +++++
5 files changed, 26 insertions(+)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index 3be17b51ca..998282e526 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -182,3 +182,11 @@ int io_channel_send(QIOChannel *ioc, const void *buf, size_t len)
{
return io_channel_send_full(ioc, buf, len, NULL, 0);
}
+
+void remove_listaner_fd_in_watch(Chardev *chr)
+{
+ ChardevClass *cc = CHARDEV_GET_CLASS(chr);
+ if (cc->chr_listener_cleanup) {
+ cc->chr_listener_cleanup(chr);
+ }
+}
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 26d2f11202..39b3a76638 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1570,6 +1570,14 @@ char_socket_get_connected(Object *obj, Error **errp)
return s->state == TCP_CHARDEV_STATE_CONNECTED;
}
+static void tcp_chr_listener_cleanup(Chardev *chr)
+{
+ SocketChardev *s = SOCKET_CHARDEV(chr);
+ if (s->listener)
+ qio_net_listener_set_client_func_full(s->listener, NULL, NULL,
+ NULL, chr->gcontext);
+}
+
static void char_socket_class_init(ObjectClass *oc, const void *data)
{
ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -1587,6 +1595,7 @@ static void char_socket_class_init(ObjectClass *oc, const void *data)
cc->chr_add_client = tcp_chr_add_client;
cc->chr_add_watch = tcp_chr_add_watch;
cc->chr_update_read_handler = tcp_chr_update_read_handler;
+ cc->chr_listener_cleanup = tcp_chr_listener_cleanup;
object_class_property_add(oc, "addr", "SocketAddress",
char_socket_get_addr, NULL,
diff --git a/include/chardev/char-io.h b/include/chardev/char-io.h
index ac379ea70e..087a250c70 100644
--- a/include/chardev/char-io.h
+++ b/include/chardev/char-io.h
@@ -43,4 +43,6 @@ int io_channel_send(QIOChannel *ioc, const void *buf, size_t len);
int io_channel_send_full(QIOChannel *ioc, const void *buf, size_t len,
int *fds, size_t nfds);
+void remove_listaner_fd_in_watch(Chardev *chr);
+
#endif /* CHAR_IO_H */
diff --git a/include/chardev/char.h b/include/chardev/char.h
index b65e9981c1..192cad67d4 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -307,6 +307,8 @@ struct ChardevClass {
/* handle various events */
void (*chr_be_event)(Chardev *s, QEMUChrEvent event);
+
+ void (*chr_listener_cleanup)(Chardev *chr);
};
Chardev *qemu_chardev_new(const char *id, const char *typename,
diff --git a/monitor/qmp.c b/monitor/qmp.c
index cb99a12d94..e2b1c49ed6 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -537,6 +537,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.
+ */
+ remove_listaner_fd_in_watch(chr);
/*
* We can't call qemu_chr_fe_set_handlers() directly here
* since chardev might be running in the monitor I/O
--
2.43.0
reply other threads:[~2025-11-22 2:44 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20251122014656.297693-1-mail@jiesong.me \
--to=mail@jiesong.me \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=eblake@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=songjie_yewu@cmss.chinamobile.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).