Hi
On Tue, Sep 30, 2025 at 3:08 PM Daniel P. Berrangé <[email protected]> wrote:
>
> The QIOChannelWebsock object releases all its resources in the
> finalize callback. This is too late, as callers expect to be
> able to call qio_channel_close() to fully close a channel and
> release resources related to I/O. Only releasing the underlying
> QIOChannel transport can be delayed until finalize.
>
> Furthermore the close callback must be robust against being
> called multiple times. Thus when moving the code we now clear
> the GSource ID using g_clear_handle_id.
>
> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
> io/channel-websock.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/io/channel-websock.c b/io/channel-websock.c
> index 0a8c5c4712..56d53355d5 100644
> --- a/io/channel-websock.c
> +++ b/io/channel-websock.c
> @@ -919,16 +919,7 @@ static void qio_channel_websock_finalize(Object *obj)
> {
> QIOChannelWebsock *ioc = QIO_CHANNEL_WEBSOCK(obj);
>
> - buffer_free(&ioc->encinput);
> - buffer_free(&ioc->encoutput);
> - buffer_free(&ioc->rawinput);
> object_unref(OBJECT(ioc->master));
> - if (ioc->io_tag) {
> - g_source_remove(ioc->io_tag);
> - }
> - if (ioc->io_err) {
> - error_free(ioc->io_err);
> - }
Maybe finalize should call close() ? Otherwise, it's hard to guarantee
that there is no leak when doing simply init/finish.
> }
>
>
> @@ -1218,6 +1209,15 @@ static int qio_channel_websock_close(QIOChannel *ioc,
> QIOChannelWebsock *wioc = QIO_CHANNEL_WEBSOCK(ioc);
>
> trace_qio_channel_websock_close(ioc);
> + buffer_free(&wioc->encinput);
> + buffer_free(&wioc->encoutput);
> + buffer_free(&wioc->rawinput);
> + if (wioc->io_tag) {
> + g_clear_handle_id(&wioc->io_tag, g_source_remove);
> + }
> + if (wioc->io_err) {
> + g_clear_pointer(&wioc->io_err, error_free);
> + }
> return qio_channel_close(wioc->master, errp);
> }
>
otherwise lgtm
> --
> 2.50.1
>
--
Marc-André Lureau
