| Message ID | alpine.DEB.2.21.2408051450540.61955@angie.orcam.me.uk |
|---|---|
| State | New |
| Headers | show |
| Series | nptl: Fix termination issues with tst-cancel7 | expand |
On 05/08/24 11:22, Maciej W. Rozycki wrote: > From: Maciej W. Rozycki <macro@redhat.com> > > Move the release of the semaphore used to synchronize between an extra > copy of the test run as a separate process and the main test process > until after the PID file has been locked. It is so that if the cleanup > function gets called by the test driver due to premature termination of > the main test process, then the function does not get at the PID file > before it has been locked and conclude that the extra copy of the test > has already terminated. This won't usually happen due to a relatively > high amount of time required to elapse before timeout triggers in the > test driver, but it will change with the next change. > > There is still a small time window remaining with this change in place > where the main test process gets killed for some reason between the > extra copy of the test has been already started by pthread_create(3) and > a successful return from the call to sem_wait(3), in which case the > cleanup function can be reached before PID has been written to the PID > file and the file locked. It seems that with the test case structured > as it is now and PID-based process management we have no means to avoid > it. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > nptl/tst-cancel7.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > glibc-tst-cancel7-sem-post.diff > Index: glibc/nptl/tst-cancel7.c > =================================================================== > --- glibc.orig/nptl/tst-cancel7.c > +++ glibc/nptl/tst-cancel7.c > @@ -57,9 +57,6 @@ sl (void) > fprintf (f, "%lld\n", (long long) getpid ()); > fflush (f); > > - if (sem_post (sem) != 0) > - FAIL_EXIT1 ("sem_post: %m"); > - > struct flock fl = > { > .l_type = F_WRLCK, > @@ -70,6 +67,9 @@ sl (void) > if (fcntl (fileno (f), F_SETLK, &fl) != 0) > FAIL_EXIT1 ("fcntl (F_SETFL): %m"); > > + if (sem_post (sem) != 0) > + FAIL_EXIT1 ("sem_post: %m"); > + > sigset_t ss; > sigfillset (&ss); > sigsuspend (&ss); > @@ -116,7 +116,7 @@ do_test (void) > { > pthread_t th = xpthread_create (NULL, tf, NULL); > > - /* Wait to cancel until after the pid is written. */ > + /* Wait to cancel until after the pid is written and file locked. */ > if (sem_wait (sem) != 0) > FAIL_EXIT1 ("sem_wait: %m"); >
Index: glibc/nptl/tst-cancel7.c =================================================================== --- glibc.orig/nptl/tst-cancel7.c +++ glibc/nptl/tst-cancel7.c @@ -57,9 +57,6 @@ sl (void) fprintf (f, "%lld\n", (long long) getpid ()); fflush (f); - if (sem_post (sem) != 0) - FAIL_EXIT1 ("sem_post: %m"); - struct flock fl = { .l_type = F_WRLCK, @@ -70,6 +67,9 @@ sl (void) if (fcntl (fileno (f), F_SETLK, &fl) != 0) FAIL_EXIT1 ("fcntl (F_SETFL): %m"); + if (sem_post (sem) != 0) + FAIL_EXIT1 ("sem_post: %m"); + sigset_t ss; sigfillset (&ss); sigsuspend (&ss); @@ -116,7 +116,7 @@ do_test (void) { pthread_t th = xpthread_create (NULL, tf, NULL); - /* Wait to cancel until after the pid is written. */ + /* Wait to cancel until after the pid is written and file locked. */ if (sem_wait (sem) != 0) FAIL_EXIT1 ("sem_wait: %m");
From: Maciej W. Rozycki <macro@redhat.com> Move the release of the semaphore used to synchronize between an extra copy of the test run as a separate process and the main test process until after the PID file has been locked. It is so that if the cleanup function gets called by the test driver due to premature termination of the main test process, then the function does not get at the PID file before it has been locked and conclude that the extra copy of the test has already terminated. This won't usually happen due to a relatively high amount of time required to elapse before timeout triggers in the test driver, but it will change with the next change. There is still a small time window remaining with this change in place where the main test process gets killed for some reason between the extra copy of the test has been already started by pthread_create(3) and a successful return from the call to sem_wait(3), in which case the cleanup function can be reached before PID has been written to the PID file and the file locked. It seems that with the test case structured as it is now and PID-based process management we have no means to avoid it. --- nptl/tst-cancel7.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) glibc-tst-cancel7-sem-post.diff