[Spice-devel] [PATCH spice-server v2] test-display-base: Avoid spurious errors due to listen failures

[Jonathon Jongsma]

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


On Fri, 2018-11-09 at 09:35 +0000, Frediano Ziglio wrote:
> To set up a listening socket usually you call in sequence:
> - socket;
> - bind;
> - listen.
> If you try to bind() to a port when another socket is already
> listening on that port, the bind() will fail.
> However, it is possible that the bind() may succeed and the listen()
> will fail, as demonstrated in the following sequence:
> - socket() create socket 1;
> - bind() to port N on socket 1;
> - socket() create socket 2;
> - bind() to port N on socket 2;
> - listen() on socket 1;
> - listen() on socket 2 <-- failure.
> 
> When running tests (especially multiple tests running in parallel),
> it
> may sometimes happen that there are other tests already listening on
> the port that we are trying to use. In this case, we want to ignore
> this error and simply try to listen on a different port. We already
> attempted to handle this scenario, but we were only ignoring bind()
> errors and not listen() errors. So in the scenario mentioned above,
> the listen() error was causing the entire test to fail instead of
> allowing us to try to listen on another port.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/tests/test-display-base.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> Changes since v1:
> - update commit message
> 
> diff --git a/server/tests/test-display-base.c b/server/tests/test-
> display-base.c
> index ea3a23ba..f58f76d3 100644
> --- a/server/tests/test-display-base.c
> +++ b/server/tests/test-display-base.c
> @@ -894,10 +894,10 @@ void test_set_command_list(Test *test, Command
> *commands, int num_commands)
>      test->num_commands = num_commands;
>  }
>  
> -static gboolean ignore_bind_failures(const gchar *log_domain,
> -                                     GLogLevelFlags log_level,
> -                                     const gchar *message,
> -                                     gpointer user_data)
> +static gboolean ignore_in_use_failures(const gchar *log_domain,
> +                                       GLogLevelFlags log_level,
> +                                       const gchar *message,
> +                                       gpointer user_data)
>  {
>      if (!g_str_equal (log_domain, G_LOG_DOMAIN)) {
>          return true;
> @@ -905,7 +905,8 @@ static gboolean ignore_bind_failures(const gchar
> *log_domain,
>      if ((log_level & G_LOG_LEVEL_WARNING) == 0)  {
>          return true;
>      }
> -    if (strstr(message, "reds_init_socket: binding socket to ") ==
> NULL) {
> +    if (strstr(message, "reds_init_socket: binding socket to ") ==
> NULL || // bind failure
> +        strstr(message, "reds_init_socket: listen: ") == NULL) { //
> listen failure
>          g_print("XXX [%s]\n", message);
>          return true;
>      }
> @@ -929,7 +930,7 @@ Test* test_new(SpiceCoreInterface* core)
>      // some common initialization for all display tests
>      port = BASE_PORT;
>  
> -    g_test_log_set_fatal_handler(ignore_bind_failures, NULL);
> +    g_test_log_set_fatal_handler(ignore_in_use_failures, NULL);
>      for (port = BASE_PORT; port < BASE_PORT + 10; port++) {
>          SpiceServer* server = spice_server_new();
>          spice_server_set_noauth(server);