Am 03.11.2025 um 21:10 hat Eric Blake geschrieben:
> 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 <[email protected]>
> @@ -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;
Now that you show me this, I think patch 2 actually also needs to check
that context is unchanged. We don't remember the old value before this
patch, so maybe the order of patch 2 and 3 should be swapped.
Kevin
