Re: [PATCH v2 07/12] qio: Hoist ref of listener outside loop

["Daniel P. Berrangé" <berrange@redhat.com>]

On Sat, Nov 08, 2025 at 04:59:28PM -0600, Eric Blake wrote:
> The point of QIONetListener is to allow a server to listen to more
> than one socket address at a time, and respond to clients in a
> first-come-first-serve order across any of those addresses.  While
> some servers (like NBD) really do want to serve multiple simultaneous
> clients, others only care about the first client to connect, and will
> immediately deregister the callback, possibly by dropping their
> reference to the QIONetListener.  The existing code ensures that the
> reference count on the listener will not drop to zero while there are
> any pending GSource (since each GSource will not call the notify of
> object_unref() until it is no longer servicing a callback); however,
> it is also possible to guarantee the same setup by merely holding an
> overall reference to the listener as long as there is at least one
> GSource, as well as also holding a local reference around any callback
> function that has not yet run to completion.
> 
> Hoisting the reference like this will make it easier for an upcoming
> patch to still ensure the listener cannot be prematurely finalized
> during the user's callback, even when using AioContext (where we have
> not plumbed in support for a notify function) rather than GSource.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v2: also grab reference around callback execution

Still not comfortable with this. I've sent a reply to your v1
patch though, so we keep the analysis in one thread.

> ---
>  io/net-listener.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/io/net-listener.c b/io/net-listener.c
> index ad097175eba..dd3522c9b3c 100644
> --- a/io/net-listener.c
> +++ b/io/net-listener.c
> @@ -54,7 +54,9 @@ static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
>      trace_qio_net_listener_callback(listener, listener->io_func,
>                                      listener->context);
>      if (listener->io_func) {
> +        object_ref(OBJECT(listener));
>          listener->io_func(listener, sioc, listener->io_data);
> +        object_unref(OBJECT(listener));
>      }
> 
>      object_unref(OBJECT(sioc));
> @@ -120,12 +122,14 @@ qio_net_listener_watch(QIONetListener *listener, size_t i, const char *caller)
> 
>      trace_qio_net_listener_watch(listener, listener->io_func,
>                                   listener->context, caller);
> -    for ( ; i < listener->nsioc; i++) {
> +    if (i == 0) {
>          object_ref(OBJECT(listener));
> +    }
> +    for ( ; i < listener->nsioc; i++) {
>          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, listener->context);
> +            listener, NULL, listener->context);
>      }
>  }
> 
> @@ -147,6 +151,7 @@ qio_net_listener_unwatch(QIONetListener *listener, const char *caller)
>              listener->io_source[i] = NULL;
>          }
>      }
> +    object_unref(OBJECT(listener));
>  }
> 
>  void qio_net_listener_add(QIONetListener *listener,
> -- 
> 2.51.1
> 

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 :|