Re: [PATCH 6/8] qio: Hoist ref of listener outside loop

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

On Mon, Nov 03, 2025 at 02:10:57PM -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, many other servers 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 all other pending callbacks remain safe once the first
> callback drops the listener, by adding an extra reference to the
> listener for each GSource created, where those references pair to the
> eventual teardown of each GSource after a given callbacks has been
> serviced or aborted.  But it is equally acceptable to hoist the
> reference to the listener outside the loop - as long as there is a
> callback function registered, it is sufficient to have a single
> reference live for the entire array of sioc, rather than one reference
> per sioc in the array.
> 
> Hoisting the reference like this will make it easier for an upcoming
> patch to still ensure the listener cannot be prematurely garbage
> collected during the user's callback, even when the callback no longer
> uses a per-sioc GSource.

It isn't quite this simple. Glib reference counts the callback
func / data, holding a reference when dispatching the callback.

IOW, even if the GSource is unrefed, the callback 'notify'
function won't be called if the main loop is in the process
of dispatching.

With this change, the reference on 'listener' can now be
released even if the callback is currently dispatching.

> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  io/net-listener.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/io/net-listener.c b/io/net-listener.c
> index afdacdd5ff4..ce29bf3c993 100644
> --- a/io/net-listener.c
> +++ b/io/net-listener.c
> @@ -118,12 +118,14 @@ qio_net_listener_watch(QIONetListener *listener, size_t i, const char *caller)
>      }
> 
>      trace_qio_net_listener_watch_enabled(listener, listener->io_func, 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);
>      }
>  }
> 
> @@ -144,6 +146,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 :|