[Spice-devel] [PATCH spice-gtk] Detect timeout conditions more aggressively on Linux

[Frediano Ziglio]

> 
> This mitigates a fairly rare problem we see with our kiosk mode clients.
> That is, normally if something goes wrong with a client connection
> (e.g. the session is killed, or the server is restarted ), the kiosk will
> exit on disconnect, and we get a chance to retry the connection, or
> present the user with a 'server down' style message.
> 
> But in the case of a serious network problem or a server hard power
> cycle (i.e. no TCP FIN packets can flow), our end user behavior is not
> ideal - the kiosk appears to hang solid, requiring a power cycle.
> 
> That's because we've got the stock keepalive timeouts, or about 2 hours
> and 11 minutes, before the client sees the disconnect.
> 
> This change will cause the client to recognize the server has vanished
> without a TCP FIN after 75 seconds.
> 
> See this thread:
>   https://lists.freedesktop.org/archives/spice-devel/2017-March/036553.html
> 

I would avoid to refer to a message on ML

> As well as this bug:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1436589
> 
> Signed-off-by: Jeremy White <jwhite at codeweavers.com>

Can I suggest this follow up?
https://gitlab.freedesktop.org/fziglio/spice-gtk/commit/e76f04625c075776f0ca0e78e5a42e4aa396faa7

> ---
>  configure.ac        | 18 ++++++++++++++++++
>  meson.build         | 15 +++++++++++++++
>  src/spice-session.c | 15 +++++++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 0666e2a8..9c64a8dd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -82,6 +82,24 @@ AM_CONDITIONAL([HAVE_EGL],[test "$have_egl" = "yes"])
>  AS_IF([test "$have_egl" = "yes"],
>         AC_DEFINE([HAVE_EGL], [1], [Define if supporting EGL]))
>  
> +AC_CHECK_DECL([TCP_KEEPIDLE], [have_tcp_keepidle="yes"],,
> +              [#include <netinet/tcp.h>])
> +AS_IF([test "x$have_tcp_keepidle" = "xyes"],
> +      [AC_DEFINE([HAVE_TCP_KEEPIDLE],1,[Define to 1 if <netinet/tcp.h> has a
> TCP_KEEPIDLE definition])],
> +)
> +AC_CHECK_DECL([TCP_KEEPINTVL], [have_tcp_keepintvl="yes"],,
> +              [#include <netinet/tcp.h>])
> +AS_IF([test "x$have_tcp_keepintvl" = "xyes"],
> +      [AC_DEFINE([HAVE_TCP_KEEPINTVL],1,[Define to 1 if <netinet/tcp.h> has
> a TCP_KEEPINTVL definition])],
> +)
> +AC_CHECK_DECL([TCP_KEEPCNT], [have_tcp_keepcnt="yes"],,
> +              [#include <netinet/tcp.h>])
> +AS_IF([test "x$have_tcp_keepcnt" = "xyes"],
> +      [AC_DEFINE([HAVE_TCP_KEEPCNT],1,[Define to 1 if <netinet/tcp.h> has a
> TCP_KEEPCNT definition])],
> +)
> +
> +
> +
>  AC_CHECK_LIBM
>  AC_SUBST(LIBM)
>  
> diff --git a/meson.build b/meson.build
> index e0fba930..f2ef94a9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -72,6 +72,21 @@ foreach func : ['clearenv', 'strtok_r']
>    endif
>  endforeach
>  
> +# TCP_KEEPIDLE definition in netinet/tcp.h
> +if compiler.has_header_symbol('netinet/tcp.h', 'TCP_KEEPIDLE')
> +  spice_gtk_config_data.set('HAVE_TCP_KEEPIDLE', '1')
> +endif
> +
> +# TCP_KEEPINTVL definition in netinet/tcp.h
> +if compiler.has_header_symbol('netinet/tcp.h', 'TCP_KEEPINTVL')
> +  spice_gtk_config_data.set('HAVE_TCP_KEEPINTVL', '1')
> +endif
> +
> +# TCP_KEEPCNT definition in netinet/tcp.h
> +if compiler.has_header_symbol('netinet/tcp.h', 'TCP_KEEPCNT')
> +  spice_gtk_config_data.set('HAVE_TCP_KEEPCNT', '1')
> +endif
> +
>  #
>  # check for mandatory dependencies
>  #
> diff --git a/src/spice-session.c b/src/spice-session.c
> index 89528970..f6ec6a83 100644
> --- a/src/spice-session.c
> +++ b/src/spice-session.c
> @@ -18,6 +18,7 @@
>  #include "config.h"
>  
>  #include <gio/gio.h>
> +#include <gio/gnetworking.h>
>  #include <glib.h>
>  #ifdef G_OS_UNIX
>  #include <gio/gunixsocketaddress.h>
> @@ -2254,6 +2255,20 @@ GSocketConnection*
> spice_session_channel_open_host(SpiceSession *session, SpiceC
>          g_socket_set_timeout(socket, 0);
>          g_socket_set_blocking(socket, FALSE);
>          g_socket_set_keepalive(socket, TRUE);
> +
> +        /* Make Linux client timeouts a bit more responsive */
> +        /*  TODO:  Support Windows.  It appears as though you can
> +                   set KEEPIDLE and KEEPINTVL, but not KEEPCNT.
> +                   See SIO_KEEPALIVE_VALS */
> +#ifdef HAVE_TCP_KEEPIDLE
> +        g_socket_set_option(socket, IPPROTO_TCP, TCP_KEEPIDLE, 30, NULL);
> +#endif
> +#ifdef HAVE_TCP_KEEPINTVL
> +        g_socket_set_option(socket, IPPROTO_TCP, TCP_KEEPINTVL, 15, NULL);
> +#endif
> +#ifdef HAVE_TCP_KEEPCNT
> +        g_socket_set_option(socket, IPPROTO_TCP, TCP_KEEPCNT, 3, NULL);
> +#endif
>      }
>  
>      g_clear_object(&open_host.client);

Frediano