diff mbox series

[05/11] test-util-sockets: Synchronize properly, don't sleep(1)

Message ID 20201029133833.3450220-6-armbru@redhat.com
State New, archived
Headers show
Series sockets: Attempt to drain the abstract socket swamp | expand

Commit Message

Markus Armbruster Oct. 29, 2020, 1:38 p.m. UTC
The abstract sockets test spawns a thread to listen and a accept, and
a second one to connect, with a sleep(1) in between to "ensure" the
former is listening when the latter tries to connect.  Review fail.
Risks spurious test failure, say when a heavily loaded machine doesn't
schedule the first thread quickly enough.  It's also slow.

Listen and accept in the main thread, and start the connect thread in
between.  Look ma, no sleep!  Run time drops from 2s wall clock to a
few milliseconds.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-util-sockets.c | 39 +++++++++++++--------------------------
 1 file changed, 13 insertions(+), 26 deletions(-)

Comments

Eric Blake Oct. 29, 2020, 6:54 p.m. UTC | #1
On 10/29/20 8:38 AM, Markus Armbruster wrote:
> The abstract sockets test spawns a thread to listen and a accept, and

s/and a/and/

> a second one to connect, with a sleep(1) in between to "ensure" the
> former is listening when the latter tries to connect.  Review fail.
> Risks spurious test failure, say when a heavily loaded machine doesn't
> schedule the first thread quickly enough.  It's also slow.
> 
> Listen and accept in the main thread, and start the connect thread in
> between.  Look ma, no sleep!  Run time drops from 2s wall clock to a
> few milliseconds.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/test-util-sockets.c | 39 +++++++++++++--------------------------
>  1 file changed, 13 insertions(+), 26 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster Oct. 30, 2020, 6:40 a.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 10/29/20 8:38 AM, Markus Armbruster wrote:
>> The abstract sockets test spawns a thread to listen and a accept, and
>
> s/and a/and/

Yes.

>> a second one to connect, with a sleep(1) in between to "ensure" the
>> former is listening when the latter tries to connect.  Review fail.
>> Risks spurious test failure, say when a heavily loaded machine doesn't
>> schedule the first thread quickly enough.  It's also slow.
>> 
>> Listen and accept in the main thread, and start the connect thread in
>> between.  Look ma, no sleep!  Run time drops from 2s wall clock to a
>> few milliseconds.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/test-util-sockets.c | 39 +++++++++++++--------------------------
>>  1 file changed, 13 insertions(+), 26 deletions(-)
>> 
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
diff mbox series

Patch

diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index 77fc51d6f5..c2802f69ee 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -230,25 +230,6 @@  static void test_socket_fd_pass_num_nocli(void)
 #endif
 
 #ifdef __linux__
-static gpointer unix_server_thread_func(gpointer user_data)
-{
-    int fd;
-    int connfd;
-    struct sockaddr_un un;
-    socklen_t len = sizeof(un);
-
-    fd = socket_listen(user_data, 1, &error_abort);
-    g_assert_cmpint(fd, >=, 0);
-    g_assert(fd_is_socket(fd));
-
-    connfd = accept(fd, (struct sockaddr *)&un, &len);
-    g_assert_cmpint(connfd, !=, -1);
-    close(connfd);
-
-    close(fd);
-    return NULL;
-}
-
 static gpointer unix_client_thread_func(gpointer user_data)
 {
     int fd;
@@ -261,20 +242,26 @@  static gpointer unix_client_thread_func(gpointer user_data)
 
 static void test_socket_unix_abstract_one(SocketAddress *addr)
 {
-    GThread *serv, *cli;
+    int fd, connfd;
+    GThread *cli;
+    struct sockaddr_un un;
+    socklen_t len = sizeof(un);
 
-    serv = g_thread_new("abstract_unix_server",
-                        unix_server_thread_func,
-                        addr);
-
-    sleep(1);
+    fd = socket_listen(addr, 1, &error_abort);
+    g_assert_cmpint(fd, >=, 0);
+    g_assert(fd_is_socket(fd));
 
     cli = g_thread_new("abstract_unix_client",
                        unix_client_thread_func,
                        addr);
 
+    connfd = accept(fd, (struct sockaddr *)&un, &len);
+    g_assert_cmpint(connfd, !=, -1);
+    close(connfd);
+
+    close(fd);
+
     g_thread_join(cli);
-    g_thread_join(serv);
 }
 
 static void test_socket_unix_abstract_good(void)