From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, berrange@redhat.com, kwolf@redhat.com
Subject: [PATCH 3/8] qio: Remember context of qio_net_listener_set_client_func_full
Date: Mon, 3 Nov 2025 14:10:54 -0600 [thread overview]
Message-ID: <20251103202849.3687643-13-eblake@redhat.com> (raw)
In-Reply-To: <20251103202849.3687643-10-eblake@redhat.com>
io/net-listener.c has two modes of use: asynchronous (the user calls
qio_net_listener_set_client_func to wake up the callback via the
global GMainContext, or qio_net_listener_set_client_func_full to wake
up the callback via the caller's own alternative GMainContext), and
synchronous (the user calls qio_net_listener_wait_client which creates
its own GMainContext and waits for the first client connection before
returning, with no need for a user's callback). But commit 938c8b79
("qio: store gsources for net listeners", v2.12.0) has a latent logic
flaw: when qio_net_listener_wait_client finishes on its temporary
context, it reverts all of the siocs back to the global GMainContext
rather than the potentially non-NULL context they might have been
originally registered with. Similarly, if the user creates a
net-listener, adds initial addresses, registers an async callback with
a non-default context (which ties to all siocs for the initial
addresses), then adds more addresses with qio_net_listener_add, the
siocs for later addresses are blindly placed in the global context,
rather than sharing the context of the earlier ones.
In practice, I don't think this has caused issues. As pointed out by
the original commit, all async callers prior to that commit were
already okay with the NULL default context; and the typical usage
pattern is to first add ALL the addresses the listener will pay
attention to before ever setting the async callback. Likewise, if a
file uses only qio_net_listener_set_client_func instead of
qio_net_listener_set_client_func_full, then it is never using a custom
context, so later assignments of async callbacks will still be to the
same global context as earlier ones. Meanwhile, any callers that want
to do the sync operation to grab the first client are unlikely to
register an async callback; altogether bypassing the question of
whether later assignments of a GSource are being tied to a different
context over time.
I do note that chardev/char-socket.c is the only file that calls both
qio_net_listener_wait_client (sync for a single client in
tcp_chr_accept_server_sync), and qio_net_listener_set_client_func_full
(several places, all with chr->gcontext, but sometimes with a NULL
callback function during teardown). But as far as I can tell, the two
uses are mutually exclusive, based on the is_waitconnect parameter to
qmp_chardev_open_socket_server.
That said, it is more robust to remember when a callback function is
tied to a non-default context, and have both the sync wait and any
late address additions honor that same context. That way, the code
will be robust even if a later user performs a sync wait for a
specific client in the middle of servicing a longer-lived
QIONetListener that has an async callback for all other clients.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/io/net-listener.h | 1 +
io/net-listener.c | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/io/net-listener.h b/include/io/net-listener.h
index ab9f291ed62..42fbfab5467 100644
--- a/include/io/net-listener.h
+++ b/include/io/net-listener.h
@@ -50,6 +50,7 @@ struct QIONetListener {
QIOChannelSocket **sioc;
GSource **io_source;
size_t nsioc;
+ GMainContext *context;
bool connected;
diff --git a/io/net-listener.c b/io/net-listener.c
index e89286ea63c..15df673fb6e 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -132,7 +132,7 @@ void qio_net_listener_add(QIONetListener *listener,
listener->io_source[listener->nsioc] = qio_channel_add_watch_source(
QIO_CHANNEL(listener->sioc[listener->nsioc]), G_IO_IN,
qio_net_listener_channel_func,
- listener, (GDestroyNotify)object_unref, NULL);
+ listener, (GDestroyNotify)object_unref, listener->context);
}
listener->nsioc++;
@@ -160,6 +160,7 @@ void qio_net_listener_set_client_func_full(QIONetListener *listener,
listener->io_func = func;
listener->io_data = data;
listener->io_notify = notify;
+ listener->context = context;
for (i = 0; i < listener->nsioc; i++) {
if (listener->io_source[i]) {
@@ -271,7 +272,7 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
listener->io_source[i] = qio_channel_add_watch_source(
QIO_CHANNEL(listener->sioc[i]), G_IO_IN,
qio_net_listener_channel_func,
- listener, (GDestroyNotify)object_unref, NULL);
+ listener, (GDestroyNotify)object_unref, listener->context);
}
}
--
2.51.1
next prev parent reply other threads:[~2025-11-03 20:31 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-03 20:10 [PATCH 0/8] Fix deadlock with bdrv_open of self-served NBD Eric Blake
2025-11-03 20:10 ` [PATCH 1/8] qio: Add trace points to net_listener Eric Blake
2025-11-04 10:43 ` Daniel P. Berrangé
2025-11-04 11:08 ` Kevin Wolf
2025-11-05 17:18 ` Eric Blake
2025-11-06 12:20 ` Kevin Wolf
2025-11-03 20:10 ` [PATCH 2/8] qio: Minor optimization when callback function is unchanged Eric Blake
2025-11-04 10:44 ` Daniel P. Berrangé
2025-11-04 11:13 ` Kevin Wolf
2025-11-05 17:23 ` Eric Blake
2025-11-03 20:10 ` Eric Blake [this message]
2025-11-04 10:50 ` [PATCH 3/8] qio: Remember context of qio_net_listener_set_client_func_full Daniel P. Berrangé
2025-11-04 11:25 ` Kevin Wolf
2025-11-05 19:18 ` Eric Blake
2025-11-03 20:10 ` [PATCH 4/8] qio: Factor out helpers qio_net_listener_[un]watch Eric Blake
2025-11-04 11:03 ` Daniel P. Berrangé
2025-11-04 13:15 ` Kevin Wolf
2025-11-05 19:22 ` Eric Blake
2025-11-04 12:37 ` Kevin Wolf
2025-11-04 13:10 ` Daniel P. Berrangé
2025-11-05 19:32 ` Eric Blake
2025-11-03 20:10 ` [PATCH 5/8] qio: Let listening sockets remember their owning QIONetListener Eric Blake
2025-11-05 20:06 ` Eric Blake
2025-11-06 18:35 ` Eric Blake
2025-11-07 8:50 ` Daniel P. Berrangé
2025-11-07 13:47 ` Eric Blake
2025-11-03 20:10 ` [PATCH 6/8] qio: Hoist ref of listener outside loop Eric Blake
2025-11-04 11:13 ` Daniel P. Berrangé
2025-11-05 21:57 ` Eric Blake
2025-11-11 14:43 ` Daniel P. Berrangé
2025-11-11 15:48 ` Kevin Wolf
2025-11-11 16:07 ` Daniel P. Berrangé
2025-11-11 19:09 ` Eric Blake
2025-11-11 20:07 ` Eric Blake
2025-11-03 20:10 ` [PATCH 7/8] qio: Use AioContext for default-context QIONetListener Eric Blake
2025-11-04 11:37 ` Daniel P. Berrangé
2025-11-05 22:06 ` Eric Blake
2025-11-04 15:14 ` Kevin Wolf
2025-11-03 20:10 ` [PATCH 8/8] iotests: Add coverage of recent NBD qio deadlock fix Eric Blake
2025-11-04 11:38 ` Vladimir Sementsov-Ogievskiy
2025-11-05 22:10 ` Eric Blake
2025-11-06 8:20 ` Vladimir Sementsov-Ogievskiy
2025-11-06 12:26 ` 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=20251103202849.3687643-13-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=berrange@redhat.com \
--cc=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 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).