This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] posix/tst-spawn: Fix racy tests in spawned processes.


On 01/28/2019 03:15 PM, Adhemerval Zanella wrote:


On 28/01/2019 11:41, Stefan Liebler wrote:
Hi,

from time to time I get fails in tst-spawn like:
tst-spawn.c:111: numeric comparison failure
    left: 0 (0x0); from: xlseek (fd2, 0, SEEK_CUR)
   right: 28 (0x1c); from: strlen (fd2string)
error: 1 test failures
tst-spawn.c:252: numeric comparison failure
    left: 1 (0x1); from: WEXITSTATUS (status)
   right: 0 (0x0); from: 0
error: 1 test failures

It turned out, that a child process is testing it's open file descriptors with e.g. a sequence of testing the current position, setting the position to zero and reading a specific amount of bytes.

Unfortunately starting with commit 2a69f853c03034c2e383e0f9c35b5402ce8b5473 the test is spawning a second child process which is sharing some of the file descriptors.  If the test sequence as mentioned above is running in parallel it leads to test failures.

As the second call of posix_spawn shall test a NULL pid argument, this patch is just moving the waitpid of the first child before the posix_spawn of the second child.

Okay for commit?
(I assume 2.30)

Bye,
Stefan

ChangeLog:

     * posix/tst-spawn do_test(): Move waitpid before posix_spawn.


It looks you forgot to add the patch itself on the message, but based on the description
I think you meant something like the below:

diff --git a/posix/tst-spawn.c b/posix/tst-spawn.c
index eea5add..2aa0dcc 100644
--- a/posix/tst-spawn.c
+++ b/posix/tst-spawn.c
@@ -237,25 +237,25 @@ do_test (int argc, char *argv[])
    TEST_COMPARE (posix_spawn (&pid, argv[1], &actions, NULL, spargv, environ),
                 0);

-  /* Same test but with a NULL pid argument.  */
-  TEST_COMPARE (posix_spawn (NULL, argv[1], &actions, NULL, spargv, environ),
-               0);
-
-  /* Cleanup.  */
-  TEST_COMPARE (posix_spawn_file_actions_destroy (&actions), 0);
-  free (name3_copy);
-
    /* Wait for the children.  */
    TEST_COMPARE (xwaitpid (pid, &status, 0), pid);
    TEST_VERIFY (WIFEXITED (status));
    TEST_VERIFY (!WIFSIGNALED (status));
    TEST_COMPARE (WEXITSTATUS (status), 0);

+  /* Same test but with a NULL pid argument.  */
+  TEST_COMPARE (posix_spawn (NULL, argv[1], &actions, NULL, spargv, environ),
+               0);
+
    xwaitpid (-1, &status, 0);
    TEST_VERIFY (WIFEXITED (status));
    TEST_VERIFY (!WIFSIGNALED (status));
    TEST_COMPARE (WEXITSTATUS (status), 0);

+  /* Cleanup.  */
+  TEST_COMPARE (posix_spawn_file_actions_destroy (&actions), 0);
+  free (name3_copy);
+
    return 0;
  }

right?

Yes, this is similar except that the "cleanup" is done before the "waitpid(-1)" as before.
See my mail in this thread with the "attached" patch.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]