[Spice-devel] [spice-server v2 5/7] reds: Close sockets when using spice_server_destroy()

[Frediano Ziglio]

> 
> Currently, the network sockets opened by reds_init_net() are not closed
> on destruction, in other words they are leaked.
> ---
>  server/reds.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/server/reds.c b/server/reds.c
> index 8ef4efe..04f0856 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3463,6 +3463,9 @@ SPICE_GNUC_VISIBLE SpiceServer *spice_server_new(void)
>  #ifdef RED_STATISTICS
>      reds->stat_file = stat_file_new(REDS_MAX_STAT_NODES);
>  #endif
> +    reds->listen_socket = -1;
> +    reds->secure_listen_socket = -1;
> +
>      return reds;
>  }
>  
> @@ -3642,6 +3645,16 @@ SPICE_GNUC_VISIBLE void
> spice_server_destroy(SpiceServer *reds)
>      if (reds->main_dispatcher) {
>          g_object_unref(reds->main_dispatcher);
>      }
> +    if (reds->listen_socket != -1) {
> +       if (reds->config->spice_listen_socket_fd != reds->listen_socket) {
> +          close(reds->listen_socket);
> +       }
> +       reds_core_watch_remove(reds, reds->listen_watch);
> +    }
> +    if (reds->secure_listen_socket != -1) {
> +       close(reds->secure_listen_socket);
> +       reds_core_watch_remove(reds, reds->secure_listen_watch);
> +    }
>  
>      reds_cleanup(reds);
>  #ifdef RED_STATISTICS

Looks good.
I would move the close after reds_core_watch_remove but it's pure paranoia
(it would matter in case another thread is handling the events).

<OT>
About the spice_listen_socket_fd != listen_socket check, it's fine
but looking at reds_init_net it seems possible to have listen_watch
assigned twice, one for an unix socket and one for spice_listen_socket_fd.
This would lead to a leak for the unix listen_watch.

Frediano