nptl: Avoid expected SIGALRM in most tests [BZ #20432]
Commit Message
Before this change, several tests did not detect early deadlocks
because they used SIGALRM as the expected signal, and they ran
for the full default TIMEOUT seconds.
This commit adds a new delayed_exit function to the test skeleton,
along with several error-checking wrappers to pthread functions.
Additional error checking is introduced into several tests.
2016-08-17 Florian Weimer <fweimer@redhat.com>
[BZ #20432]
Avoid expected SIGALRM signals.
* test-skeleton.c (xpthread_sigmask, xpthread_mutex_lock)
(xpthread_spin_lock, xpthread_cond_wait, xpthread_barrier_wait)
(xpthread_create, xpthread_detach, xpthread_join)
(delayed_exit_thread, delayed_exit): New functions.
* nptl/tst-cond3 (EXPECTED_SIGNAL): Remove.
(tf): Use xpthread_cond_wait.
(do_test): Likewise. Replace alarm with delayed_exit.
* nptl/tst-eintr1.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
(do_test): Call delayed_exit. Report failure.
* nptl/tst-eintr2.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
(do_test): Call delayed_exit.
* nptl/tst-eintr3.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
(do_test): Call delayed_exit. Use xpthread_join. Report error.
* nptl/tst-eintr4.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
(do_test): Call delayed_exit. Use xpthread_barrier_wait. Report
error.
* nptl/tst-eintr5.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
(do_test): Call delayed_exit. Use xpthread_cond_wait. Report
error.
* nptl/tst-exit2.c (EXPECTED_SIGNAL): Remove.
(do_test): Call delayed_exit.
* nptl/tst-exit3.c (EXPECTED_SIGNAL): Remove.
(do_test): Call delayed_exit.
* nptl/tst-mutex6.c (EXPECTED_SIGNAL): Remove.
(do_test): Call delayed_exit instead of alarm. Use
xpthread_mutex_lock.
* nptl/tst-rwlock5.c (EXPECTED_SIGNAL): Remove.
(do_test): Call delayed_exit instead of alarm. Use
xpthread_mutex_lock.
* nptl/tst-sem2.c (EXPECTED_SIGNAL): Remove.
(do_test): Call delayed_exit instead of alarm.
* nptl/tst-spin3.c (EXPECTED_SIGNAL): Remove.
(do_test): Call delayed_exit instead of alarm. Use
xpthread_spin_lock.
* nptl/tst-stdio1.c (EXPECTED_SIGNAL): Remove.
(do_test): Call delayed_exit instead of alarm. Use
xpthread_join.
Comments
On 08/17/2016 08:54 AM, Florian Weimer wrote:
> Before this change, several tests did not detect early deadlocks
> because they used SIGALRM as the expected signal, and they ran
> for the full default TIMEOUT seconds.
>
> This commit adds a new delayed_exit function to the test skeleton,
> along with several error-checking wrappers to pthread functions.
> Additional error checking is introduced into several tests.
This is much better than what we have now, so please commit this.
My comments are purely optional and summarized as:
- Please try to use "error:" prefix for all error messages, adding
some more, and cleaning up others.
- Might we consider robust xpthread_create handling in the face of
EAGAIN.
Overall I think this patch is a step in the right direction.
I still dislike timed tests because you have no real guarantee that,
for example in tst-eintr1, that the EINTR source thread has delivered
any signals at all. The testing should verify that at least N signals
have been delivered to the processes in question, but that's harder
and certainly out of scope.
This patch is OK.
> 2016-08-17 Florian Weimer <fweimer@redhat.com>
>
> [BZ #20432]
> Avoid expected SIGALRM signals.
> * test-skeleton.c (xpthread_sigmask, xpthread_mutex_lock)
> (xpthread_spin_lock, xpthread_cond_wait, xpthread_barrier_wait)
> (xpthread_create, xpthread_detach, xpthread_join)
> (delayed_exit_thread, delayed_exit): New functions.
> * nptl/tst-cond3 (EXPECTED_SIGNAL): Remove.
> (tf): Use xpthread_cond_wait.
> (do_test): Likewise. Replace alarm with delayed_exit.
> * nptl/tst-eintr1.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
> (do_test): Call delayed_exit. Report failure.
> * nptl/tst-eintr2.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
> (do_test): Call delayed_exit.
> * nptl/tst-eintr3.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
> (do_test): Call delayed_exit. Use xpthread_join. Report error.
> * nptl/tst-eintr4.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
> (do_test): Call delayed_exit. Use xpthread_barrier_wait. Report
> error.
> * nptl/tst-eintr5.c (EXPECTED_SIGNAL, TIMEOUT): Remove.
> (do_test): Call delayed_exit. Use xpthread_cond_wait. Report
> error.
> * nptl/tst-exit2.c (EXPECTED_SIGNAL): Remove.
> (do_test): Call delayed_exit.
> * nptl/tst-exit3.c (EXPECTED_SIGNAL): Remove.
> (do_test): Call delayed_exit.
> * nptl/tst-mutex6.c (EXPECTED_SIGNAL): Remove.
> (do_test): Call delayed_exit instead of alarm. Use
> xpthread_mutex_lock.
> * nptl/tst-rwlock5.c (EXPECTED_SIGNAL): Remove.
> (do_test): Call delayed_exit instead of alarm. Use
> xpthread_mutex_lock.
> * nptl/tst-sem2.c (EXPECTED_SIGNAL): Remove.
> (do_test): Call delayed_exit instead of alarm.
> * nptl/tst-spin3.c (EXPECTED_SIGNAL): Remove.
> (do_test): Call delayed_exit instead of alarm. Use
> xpthread_spin_lock.
> * nptl/tst-stdio1.c (EXPECTED_SIGNAL): Remove.
> (do_test): Call delayed_exit instead of alarm. Use
> xpthread_join.
>
> diff --git a/nptl/tst-cond3.c b/nptl/tst-cond3.c
> index ff904e4..724630f 100644
> --- a/nptl/tst-cond3.c
> +++ b/nptl/tst-cond3.c
> @@ -22,6 +22,10 @@
> #include <string.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> /* Note that this test requires more than the standard. It is
> required that there are no spurious wakeups if only more readers
> @@ -50,7 +54,8 @@ tf (void *arg)
> }
>
> /* This call should never return. */
> - pthread_cond_wait (&cond, &mut);
> + xpthread_cond_wait (&cond, &mut);
> + puts ("error: pthread_cond_wait in tf returned");
OK.
>
> /* We should never get here. */
> exit (1);
> @@ -96,17 +101,11 @@ do_test (void)
> }
> }
>
> - /* Set an alarm for 1 second. The wrapper will expect this. */
> - alarm (1);
> + delayed_exit (1);
>
> /* This call should never return. */
> - pthread_cond_wait (&cond, &mut);
> + xpthread_cond_wait (&cond, &mut);
>
> - puts ("cond_wait returned");
> + puts ("pthread_cond_wait in do_test returned");
Add "error:"
> return 1;
> }
> -
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-eintr1.c b/nptl/tst-eintr1.c
> index 11cd876..0946894 100644
> --- a/nptl/tst-eintr1.c
> +++ b/nptl/tst-eintr1.c
> @@ -23,6 +23,11 @@
> #include <stdlib.h>
> #include <string.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> #include "eintr.c"
>
>
> @@ -92,13 +97,8 @@ do_test (void)
> }
> }
>
> + delayed_exit (3);
> + /* This call must never return. */
> (void) tf1 (NULL);
> - /* NOTREACHED */
> -
Print a failure message please.
> - return 0;
> + return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TIMEOUT 3
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-eintr2.c b/nptl/tst-eintr2.c
> index 7f50f4b..0ef6d30 100644
> --- a/nptl/tst-eintr2.c
> +++ b/nptl/tst-eintr2.c
> @@ -24,6 +24,11 @@
> #include <string.h>
> #include <sys/time.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> #include "eintr.c"
>
>
> @@ -103,6 +108,7 @@ do_test (void)
> exit (1);
> }
>
> + delayed_exit (3);
> /* This call must never return. */
> e = pthread_mutex_lock (&m1);
> printf ("main: mutex_lock returned: %s\n",
Would be nice to add "error:" to the front of this.
> @@ -110,8 +116,3 @@ do_test (void)
>
> return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TIMEOUT 3
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-eintr3.c b/nptl/tst-eintr3.c
> index d2c32b9..f6b2ccf 100644
> --- a/nptl/tst-eintr3.c
> +++ b/nptl/tst-eintr3.c
> @@ -23,6 +23,11 @@
> #include <stdlib.h>
> #include <string.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> #include "eintr.c"
>
>
> @@ -56,16 +61,9 @@ do_test (void)
> exit (1);
> }
>
> + delayed_exit (1);
> /* This call must never return. */
> - e = pthread_join (th, NULL);
> -
> - if (e == EINTR)
> - puts ("pthread_join returned with EINTR");
> -
> - return 0;
> + xpthread_join (th);
> + puts ("error: pthread_join returned");
Likewise "error: ..."
> + return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TIMEOUT 1
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-eintr4.c b/nptl/tst-eintr4.c
> index 25fae34..f6d068f 100644
> --- a/nptl/tst-eintr4.c
> +++ b/nptl/tst-eintr4.c
> @@ -23,6 +23,11 @@
> #include <stdlib.h>
> #include <string.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> #include "eintr.c"
>
>
> @@ -40,16 +45,9 @@ do_test (void)
> exit (1);
> }
>
> + delayed_exit (1);
> /* This call must never return. */
> - int e = pthread_barrier_wait (&b);
> -
> - if (e == EINTR)
> - puts ("pthread_join returned with EINTR");
> -
> - return 0;
> + xpthread_barrier_wait (&b);
> + puts ("error: pthread_barrier_wait returned");
Likewise.
> + return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TIMEOUT 1
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-eintr5.c b/nptl/tst-eintr5.c
> index be6731c..81f900e 100644
> --- a/nptl/tst-eintr5.c
> +++ b/nptl/tst-eintr5.c
> @@ -24,6 +24,11 @@
> #include <string.h>
> #include <sys/time.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> #include "eintr.c"
>
>
> @@ -66,15 +71,9 @@ do_test (void)
> exit (1);
> }
>
> + delayed_exit (3);
> /* This call must never return. */
> - e = pthread_cond_wait (&c, &m);
> - printf ("main: cond_wait returned: %s\n",
> - strerror_r (e, buf, sizeof (buf)));
> -
> - return 0;
> + xpthread_cond_wait (&c, &m);
> + puts ("error: pthread_cond_wait returned");
> + return 1;
Likewise.
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TIMEOUT 3
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-exit2.c b/nptl/tst-exit2.c
> index 3f5ff27..0b7a2ca 100644
> --- a/nptl/tst-exit2.c
> +++ b/nptl/tst-exit2.c
> @@ -4,6 +4,10 @@
> #include <string.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> static void *
> tf (void *arg)
> @@ -28,13 +32,11 @@ do_test (void)
> return 1;
> }
>
> + delayed_exit (1);
> +
Add failure message.
> /* Terminate only this thread. */
> pthread_exit (NULL);
>
> /* NOTREACHED */
> return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-exit3.c b/nptl/tst-exit3.c
> index da92c82..9481ed9 100644
> --- a/nptl/tst-exit3.c
> +++ b/nptl/tst-exit3.c
> @@ -5,6 +5,10 @@
> #include <string.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> static pthread_barrier_t b;
>
> @@ -69,13 +73,11 @@ do_test (void)
> exit (1);
> }
>
> + delayed_exit (3);
> +
Add failure message.
> /* Terminate only this thread. */
> pthread_exit (NULL);
>
> /* NOTREACHED */
> return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-mutex6.c b/nptl/tst-mutex6.c
> index 1940687..3e989d8 100644
> --- a/nptl/tst-mutex6.c
> +++ b/nptl/tst-mutex6.c
> @@ -23,6 +23,11 @@
> #include <errno.h>
> #include <stdbool.h>
>
> +#ifndef TEST_FUNCTION
> +static int do_test (void);
> +# define TEST_FUNCTION do_test ()
> +#endif
> +#include "../test-skeleton.c"
>
> #ifndef ATTR
> pthread_mutexattr_t *attr;
> @@ -62,18 +67,10 @@ do_test (void)
> return 1;
> }
>
> - /* Set an alarm for 1 second. The wrapper will expect this. */
> - alarm (1);
> -
> + delayed_exit (1);
> /* This call should never return. */
> - pthread_mutex_lock (&m);
> + xpthread_mutex_lock (&m);
Add failure message.
>
> puts ("2nd mutex_lock returned");
> return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#ifndef TEST_FUNCTION
> -# define TEST_FUNCTION do_test ()
> -#endif
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-rwlock5.c b/nptl/tst-rwlock5.c
> index b6c5d8a..20fb471 100644
> --- a/nptl/tst-rwlock5.c
> +++ b/nptl/tst-rwlock5.c
> @@ -22,6 +22,10 @@
> #include <stdlib.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> static pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
> static pthread_rwlock_t r;
> @@ -65,22 +69,16 @@ do_test (void)
> return 1;
> }
>
> - /* Set an alarm for 1 second. The wrapper will expect this. */
> - alarm (1);
> -
> if (pthread_create (&th, NULL, tf, NULL) != 0)
> {
> puts ("create failed");
> return 1;
> }
>
> + delayed_exit (1);
> /* This call should never return. */
> - pthread_mutex_lock (&m);
> + xpthread_mutex_lock (&m);
>
> puts ("2nd mutex_lock returned");
Use error: in failure message.
> return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-sem2.c b/nptl/tst-sem2.c
> index 301dde7..1f609fc 100644
> --- a/nptl/tst-sem2.c
> +++ b/nptl/tst-sem2.c
> @@ -17,11 +17,16 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <errno.h>
> +#include <pthread.h>
> #include <semaphore.h>
> #include <signal.h>
> #include <stdio.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> static int
> do_test (void)
> @@ -34,8 +39,7 @@ do_test (void)
> return 1;
> }
>
> - /* Set an alarm for 1 second. The wrapper will expect this. */
> - alarm (1);
> + delayed_exit (1);
>
> if (TEMP_FAILURE_RETRY (sem_wait (&s)) == -1)
> {
> @@ -47,7 +51,3 @@ do_test (void)
> puts ("wait succeeded");
> return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-spin3.c b/nptl/tst-spin3.c
> index 8964ecc..6cb6f9d 100644
> --- a/nptl/tst-spin3.c
> +++ b/nptl/tst-spin3.c
> @@ -21,6 +21,10 @@
> #include <stdio.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> static int
> do_test (void)
> @@ -39,16 +43,11 @@ do_test (void)
> return 1;
> }
>
> - /* Set an alarm for 1 second. The wrapper will expect this. */
> - alarm (1);
> + delayed_exit (1);
>
> /* This call should never return. */
> - pthread_spin_lock (&s);
> + xpthread_spin_lock (&s);
>
> puts ("2nd spin_lock returned");
Use error: in failure message.
> return 1;
> }
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/nptl/tst-stdio1.c b/nptl/tst-stdio1.c
> index 2d44c16..4250e53 100644
> --- a/nptl/tst-stdio1.c
> +++ b/nptl/tst-stdio1.c
> @@ -21,6 +21,10 @@
> #include <stdio.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> static void *tf (void *a)
> {
> @@ -43,14 +47,10 @@ do_test (void)
> _exit (1);
> }
>
> - pthread_join (th, NULL);
> + delayed_exit (1);
> + xpthread_join (th);
>
> puts ("join returned");
Use error: in failure message.
>
> - return 0;
> + return 1;
> }
> -
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/test-skeleton.c b/test-skeleton.c
> index 5a90c65..b24ce1d 100644
> --- a/test-skeleton.c
> +++ b/test-skeleton.c
> @@ -559,3 +559,160 @@ main (int argc, char *argv[])
> #endif
> }
> }
I like the xFUNC variants. Thanks for this.
> +
> +/* The following functionality is only available if <pthread.h> was
> + included before this file. */
> +#ifdef _PTHREAD_H
> +
> +/* Call pthread_sigmask with error checking. */
> +static void
> +xpthread_sigmask (int how, const sigset_t *set, sigset_t *oldset)
> +{
> + if (pthread_sigmask (how, set, oldset) != 0)
> + {
> + write_message ("error: pthread_setmask failed\n");
> + _exit (1);
OK.
> + }
> +}
> +
> +/* Call pthread_mutex_lock with error checking. */
> +__attribute__ ((unused))
> +static void
> +xpthread_mutex_lock (pthread_mutex_t *mutex)
> +{
> + int ret = pthread_mutex_lock (mutex);
> + if (ret != 0)
> + {
> + errno = ret;
> + printf ("error: pthread_mutex_lock: %m\n");
> + exit (1);
OK.
> + }
> +}
> +
> +/* Call pthread_spin_lock with error checking. */
> +__attribute__ ((unused))
> +static void
> +xpthread_spin_lock (pthread_spinlock_t *lock)
> +{
> + int ret = pthread_spin_lock (lock);
> + if (ret != 0)
> + {
> + errno = ret;
> + printf ("error: pthread_spin_lock: %m\n");
> + exit (1);
OK.
> + }
> +}
> +
> +/* Call pthread_cond_wait with error checking. */
> +__attribute__ ((unused))
> +static void
> +xpthread_cond_wait (pthread_cond_t * cond,
> + pthread_mutex_t * mutex)
> +{
> + int ret = pthread_cond_wait (cond, mutex);
> + if (ret != 0)
> + {
> + errno = ret;
> + printf ("error: pthread_cond_wait: %m\n");
> + exit (1);
OK.
> + }
> +}
> +
> +/* Call pthread_barrier_wait with error checking. */
> +__attribute__ ((unused))
> +static int
> +xpthread_barrier_wait (pthread_barrier_t *barrier)
> +{
> + int ret = pthread_barrier_wait (barrier);
> + if (ret != 0 && ret != PTHREAD_BARRIER_SERIAL_THREAD)
> + {
> + errno = ret;
> + printf ("error: pthread_barrier_wait: %m\n");
> + exit (1);
> + }
> + return ret;
Ok.
> +}
> +
> +/* Call pthread_create with error checking. */
> +static pthread_t
> +xpthread_create (pthread_attr_t *attr,
> + void *(*thread_func) (void *), void *closure)
> +{
> + pthread_t thr;
> + int ret = pthread_create (&thr, attr, thread_func, closure);
> + if (ret != 0)
Could we make this more robust by looping on EAGAIN and trying
to create the thread until it succeeds or we timeout the test?
This way embedded systems with less memory might operate correctly,
and we might avoid the slow reaping problems in the linux kernel?
> + {
> + errno = ret;
> + printf ("error: pthread_create: %m\n");
> + exit (1);
> + }
> + return thr;
> +}
> +
> +/* Call pthread_detach with error checking. */
> +static void
> +xpthread_detach (pthread_t thr)
> +{
> + int ret = pthread_detach (thr);
> + if (ret != 0)
> + {
> + errno = ret;
> + printf ("error: pthread_detach: %m\n");
> + exit (1);
> + }
Ok.
> +}
> +
> +/* Call pthread_join with error checking. */
> +__attribute__ ((unused))
> +static void *
> +xpthread_join (pthread_t thr)
> +{
> + void *result;
> + int ret = pthread_join (thr, &result);
> + if (ret != 0)
> + {
> + errno = ret;
> + printf ("error: pthread_join: %m\n");
> + exit (1);
> + }
> + return result;
OK.
> +}
> +
> +/* Used to implement the delayed_exit function defined below. */
> +static void *
> +delayed_exit_thread (void *seconds_as_ptr)
> +{
> + int seconds = (uintptr_t) seconds_as_ptr;
> + struct timespec delay = { seconds, 0 };
> + struct timespec remaining = {};
> + if (nanosleep (&delay, &remaining) != 0)
> + {
> + printf ("error: nanosleep: %m\n");
> + _exit (1);
> + }
> + /* Exit the process sucessfully. */
> + exit (0);
> + return NULL;
OK.
> +}
> +
> +/* Exit (with status 0) after SECONDS have elapsed, from a helper
> + thread. The process is terminated with the exit function, so
> + atexit handlers are executed. */
> +__attribute__ ((unused))
> +static void
> +delayed_exit (int seconds)
> +{
> + /* Create the new thread with all signals blocked. */
> + sigset_t all_blocked;
> + sigfillset (&all_blocked);
> + sigset_t old_set;
> + xpthread_sigmask (SIG_SETMASK, &all_blocked, &old_set);
> + /* Create a detached thread. */
> + pthread_t thr = xpthread_create
> + (NULL, delayed_exit_thread, (void *) (uintptr_t) seconds);
> + xpthread_detach (thr);
> + /* Restore the original signal mask. */
> + xpthread_sigmask (SIG_SETMASK, &old_set, NULL);
OK.
> +}
> +
> +#endif /* _PTHREAD_H */
>
On 08/26/2016 07:01 PM, Carlos O'Donell wrote:
>> > + pthread_t thr;
>> > + int ret = pthread_create (&thr, attr, thread_func, closure);
>> > + if (ret != 0)
> Could we make this more robust by looping on EAGAIN and trying
> to create the thread until it succeeds or we timeout the test?
>
> This way embedded systems with less memory might operate correctly,
> and we might avoid the slow reaping problems in the linux kernel?
This would be undesirable because it would paper over glibc or kernel
bugs. Such bugs typically involve races in RLIMIT_NPROC accouting, for
example:
<https://sourceware.org/ml/libc-alpha/2012-09/msg00538.html>
<https://sourceware.org/ml/libc-alpha/2013-04/msg00294.html>
I believe this has subsequently been fixed in the kernel, but it appears
to have resurfaced in Linux 4.6+ in some configurations (far fewer than
before).
Florian
On 26/08/2016 14:13, Florian Weimer wrote:
> On 08/26/2016 07:01 PM, Carlos O'Donell wrote:
>>> > + pthread_t thr;
>>> > + int ret = pthread_create (&thr, attr, thread_func, closure);
>>> > + if (ret != 0)
>> Could we make this more robust by looping on EAGAIN and trying
>> to create the thread until it succeeds or we timeout the test?
>>
>> This way embedded systems with less memory might operate correctly,
>> and we might avoid the slow reaping problems in the linux kernel?
>
> This would be undesirable because it would paper over glibc or kernel bugs. Such bugs typically involve races in RLIMIT_NPROC accouting, for example:
>
> <https://sourceware.org/ml/libc-alpha/2012-09/msg00538.html>
> <https://sourceware.org/ml/libc-alpha/2013-04/msg00294.html>
>
> I believe this has subsequently been fixed in the kernel, but it appears to have resurfaced in Linux 4.6+ in some configurations (far fewer than before).
>
> Florian
>
As a side note, how this issue is related to BZ# 20432 (malloc:
Minimize interface required for interposition)?
On 26/08/2016 14:47, Adhemerval Zanella wrote:
>
>
> On 26/08/2016 14:13, Florian Weimer wrote:
>> On 08/26/2016 07:01 PM, Carlos O'Donell wrote:
>>>>> + pthread_t thr;
>>>>> + int ret = pthread_create (&thr, attr, thread_func, closure);
>>>>> + if (ret != 0)
>>> Could we make this more robust by looping on EAGAIN and trying
>>> to create the thread until it succeeds or we timeout the test?
>>>
>>> This way embedded systems with less memory might operate correctly,
>>> and we might avoid the slow reaping problems in the linux kernel?
>>
>> This would be undesirable because it would paper over glibc or kernel bugs. Such bugs typically involve races in RLIMIT_NPROC accouting, for example:
>>
>> <https://sourceware.org/ml/libc-alpha/2012-09/msg00538.html>
>> <https://sourceware.org/ml/libc-alpha/2013-04/msg00294.html>
>>
>> I believe this has subsequently been fixed in the kernel, but it appears to have resurfaced in Linux 4.6+ in some configurations (far fewer than before).
>>
>> Florian
>>
>
> As a side note, how this issue is related to BZ# 20432 (malloc:
> Minimize interface required for interposition)?
>
I just noted you corrected the bug reference in CL, nevermind then.
On Aug 17 2016, fweimer@redhat.com (Florian Weimer) wrote:
> diff --git a/nptl/tst-stdio1.c b/nptl/tst-stdio1.c
> index 2d44c16..4250e53 100644
> --- a/nptl/tst-stdio1.c
> +++ b/nptl/tst-stdio1.c
> @@ -21,6 +21,10 @@
> #include <stdio.h>
> #include <unistd.h>
>
> +static int do_test (void);
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
>
> static void *tf (void *a)
> {
> @@ -43,14 +47,10 @@ do_test (void)
> _exit (1);
> }
>
> - pthread_join (th, NULL);
> + delayed_exit (1);
> + xpthread_join (th);
>
> puts ("join returned");
>
> - return 0;
> + return 1;
> }
> -
> -
> -#define EXPECTED_SIGNAL SIGALRM
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
That doesn't work, the test now always times out because exit blocks on
flockfile.
Andreas.
On 12/15/2016 04:55 PM, Andreas Schwab wrote:
> On Aug 17 2016, fweimer@redhat.com (Florian Weimer) wrote:
>
>> diff --git a/nptl/tst-stdio1.c b/nptl/tst-stdio1.c
>> index 2d44c16..4250e53 100644
>> --- a/nptl/tst-stdio1.c
>> +++ b/nptl/tst-stdio1.c
>> @@ -21,6 +21,10 @@
>> #include <stdio.h>
>> #include <unistd.h>
>>
>> +static int do_test (void);
>> +
>> +#define TEST_FUNCTION do_test ()
>> +#include "../test-skeleton.c"
>>
>> static void *tf (void *a)
>> {
>> @@ -43,14 +47,10 @@ do_test (void)
>> _exit (1);
>> }
>>
>> - pthread_join (th, NULL);
>> + delayed_exit (1);
>> + xpthread_join (th);
>>
>> puts ("join returned");
>>
>> - return 0;
>> + return 1;
>> }
>> -
>> -
>> -#define EXPECTED_SIGNAL SIGALRM
>> -#define TEST_FUNCTION do_test ()
>> -#include "../test-skeleton.c"
>
> That doesn't work, the test now always times out because exit blocks on
> flockfile.
The test previously called exit from the signal handler. Why did it
work before?
And I don't see this blocking behavior.
Thanks,
Florian
On Dez 15 2016, Florian Weimer <fweimer@redhat.com> wrote:
> The test previously called exit from the signal handler. Why did it work
> before?
That only "works" because _IO_cleanup is not thread-safe.
Andreas.
On 03/15/2017 02:15 PM, Andreas Schwab wrote:
> On Dez 15 2016, Florian Weimer <fweimer@redhat.com> wrote:
>
>> The test previously called exit from the signal handler. Why did it work
>> before?
>
> That only "works" because _IO_cleanup is not thread-safe.
Sorry, I don't see the test failure. Based on the comments in
libio/genops.c, I think there is an expectation that even with locked
stdout, exit does not block.
In other words, if this test times out for you, this is a genuine bug,
not an issue with this test.
I may be missing something, but in that case, please elaborate.
Thanks,
Florian
On Mär 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
> On 03/15/2017 02:15 PM, Andreas Schwab wrote:
>> On Dez 15 2016, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> The test previously called exit from the signal handler. Why did it work
>>> before?
>>
>> That only "works" because _IO_cleanup is not thread-safe.
>
> Sorry, I don't see the test failure. Based on the comments in
> libio/genops.c, I think there is an expectation that even with locked
> stdout, exit does not block.
POSIX does not guarantee this.
http://austingroupbugs.net/view.php?id=611
> I may be missing something, but in that case, please elaborate.
You are not really missing anything, since you are not using this patch.
https://build.opensuse.org/package/view_file/Base:System/glibc/fix-locking-in-_IO_cleanup.patch?expand=1
Andreas.
On 03/15/2017 03:20 PM, Andreas Schwab wrote:
> On Mär 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
>
>> On 03/15/2017 02:15 PM, Andreas Schwab wrote:
>>> On Dez 15 2016, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>>> The test previously called exit from the signal handler. Why did it work
>>>> before?
>>>
>>> That only "works" because _IO_cleanup is not thread-safe.
>>
>> Sorry, I don't see the test failure. Based on the comments in
>> libio/genops.c, I think there is an expectation that even with locked
>> stdout, exit does not block.
>
> POSIX does not guarantee this.
>
> http://austingroupbugs.net/view.php?id=611
Right, but I think the intent of the upstream libio implementation is to
support this (i.e., no deadlock).
>> I may be missing something, but in that case, please elaborate.
>
> You are not really missing anything, since you are not using this patch.
>
> https://build.opensuse.org/package/view_file/Base:System/glibc/fix-locking-in-_IO_cleanup.patch?expand=1
I think you should disable this test in the patch, or check in the test
that the deadlock happens.
Thanks,
Florian
@@ -22,6 +22,10 @@
#include <string.h>
#include <unistd.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
/* Note that this test requires more than the standard. It is
required that there are no spurious wakeups if only more readers
@@ -50,7 +54,8 @@ tf (void *arg)
}
/* This call should never return. */
- pthread_cond_wait (&cond, &mut);
+ xpthread_cond_wait (&cond, &mut);
+ puts ("error: pthread_cond_wait in tf returned");
/* We should never get here. */
exit (1);
@@ -96,17 +101,11 @@ do_test (void)
}
}
- /* Set an alarm for 1 second. The wrapper will expect this. */
- alarm (1);
+ delayed_exit (1);
/* This call should never return. */
- pthread_cond_wait (&cond, &mut);
+ xpthread_cond_wait (&cond, &mut);
- puts ("cond_wait returned");
+ puts ("pthread_cond_wait in do_test returned");
return 1;
}
-
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -23,6 +23,11 @@
#include <stdlib.h>
#include <string.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
#include "eintr.c"
@@ -92,13 +97,8 @@ do_test (void)
}
}
+ delayed_exit (3);
+ /* This call must never return. */
(void) tf1 (NULL);
- /* NOTREACHED */
-
- return 0;
+ return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TIMEOUT 3
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -24,6 +24,11 @@
#include <string.h>
#include <sys/time.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
#include "eintr.c"
@@ -103,6 +108,7 @@ do_test (void)
exit (1);
}
+ delayed_exit (3);
/* This call must never return. */
e = pthread_mutex_lock (&m1);
printf ("main: mutex_lock returned: %s\n",
@@ -110,8 +116,3 @@ do_test (void)
return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TIMEOUT 3
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -23,6 +23,11 @@
#include <stdlib.h>
#include <string.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
#include "eintr.c"
@@ -56,16 +61,9 @@ do_test (void)
exit (1);
}
+ delayed_exit (1);
/* This call must never return. */
- e = pthread_join (th, NULL);
-
- if (e == EINTR)
- puts ("pthread_join returned with EINTR");
-
- return 0;
+ xpthread_join (th);
+ puts ("error: pthread_join returned");
+ return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TIMEOUT 1
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -23,6 +23,11 @@
#include <stdlib.h>
#include <string.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
#include "eintr.c"
@@ -40,16 +45,9 @@ do_test (void)
exit (1);
}
+ delayed_exit (1);
/* This call must never return. */
- int e = pthread_barrier_wait (&b);
-
- if (e == EINTR)
- puts ("pthread_join returned with EINTR");
-
- return 0;
+ xpthread_barrier_wait (&b);
+ puts ("error: pthread_barrier_wait returned");
+ return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TIMEOUT 1
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -24,6 +24,11 @@
#include <string.h>
#include <sys/time.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
#include "eintr.c"
@@ -66,15 +71,9 @@ do_test (void)
exit (1);
}
+ delayed_exit (3);
/* This call must never return. */
- e = pthread_cond_wait (&c, &m);
- printf ("main: cond_wait returned: %s\n",
- strerror_r (e, buf, sizeof (buf)));
-
- return 0;
+ xpthread_cond_wait (&c, &m);
+ puts ("error: pthread_cond_wait returned");
+ return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TIMEOUT 3
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -4,6 +4,10 @@
#include <string.h>
#include <unistd.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
static void *
tf (void *arg)
@@ -28,13 +32,11 @@ do_test (void)
return 1;
}
+ delayed_exit (1);
+
/* Terminate only this thread. */
pthread_exit (NULL);
/* NOTREACHED */
return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -5,6 +5,10 @@
#include <string.h>
#include <unistd.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
static pthread_barrier_t b;
@@ -69,13 +73,11 @@ do_test (void)
exit (1);
}
+ delayed_exit (3);
+
/* Terminate only this thread. */
pthread_exit (NULL);
/* NOTREACHED */
return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -23,6 +23,11 @@
#include <errno.h>
#include <stdbool.h>
+#ifndef TEST_FUNCTION
+static int do_test (void);
+# define TEST_FUNCTION do_test ()
+#endif
+#include "../test-skeleton.c"
#ifndef ATTR
pthread_mutexattr_t *attr;
@@ -62,18 +67,10 @@ do_test (void)
return 1;
}
- /* Set an alarm for 1 second. The wrapper will expect this. */
- alarm (1);
-
+ delayed_exit (1);
/* This call should never return. */
- pthread_mutex_lock (&m);
+ xpthread_mutex_lock (&m);
puts ("2nd mutex_lock returned");
return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#ifndef TEST_FUNCTION
-# define TEST_FUNCTION do_test ()
-#endif
-#include "../test-skeleton.c"
@@ -22,6 +22,10 @@
#include <stdlib.h>
#include <unistd.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
static pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
static pthread_rwlock_t r;
@@ -65,22 +69,16 @@ do_test (void)
return 1;
}
- /* Set an alarm for 1 second. The wrapper will expect this. */
- alarm (1);
-
if (pthread_create (&th, NULL, tf, NULL) != 0)
{
puts ("create failed");
return 1;
}
+ delayed_exit (1);
/* This call should never return. */
- pthread_mutex_lock (&m);
+ xpthread_mutex_lock (&m);
puts ("2nd mutex_lock returned");
return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -17,11 +17,16 @@
<http://www.gnu.org/licenses/>. */
#include <errno.h>
+#include <pthread.h>
#include <semaphore.h>
#include <signal.h>
#include <stdio.h>
#include <unistd.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
static int
do_test (void)
@@ -34,8 +39,7 @@ do_test (void)
return 1;
}
- /* Set an alarm for 1 second. The wrapper will expect this. */
- alarm (1);
+ delayed_exit (1);
if (TEMP_FAILURE_RETRY (sem_wait (&s)) == -1)
{
@@ -47,7 +51,3 @@ do_test (void)
puts ("wait succeeded");
return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -21,6 +21,10 @@
#include <stdio.h>
#include <unistd.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
static int
do_test (void)
@@ -39,16 +43,11 @@ do_test (void)
return 1;
}
- /* Set an alarm for 1 second. The wrapper will expect this. */
- alarm (1);
+ delayed_exit (1);
/* This call should never return. */
- pthread_spin_lock (&s);
+ xpthread_spin_lock (&s);
puts ("2nd spin_lock returned");
return 1;
}
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -21,6 +21,10 @@
#include <stdio.h>
#include <unistd.h>
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
static void *tf (void *a)
{
@@ -43,14 +47,10 @@ do_test (void)
_exit (1);
}
- pthread_join (th, NULL);
+ delayed_exit (1);
+ xpthread_join (th);
puts ("join returned");
- return 0;
+ return 1;
}
-
-
-#define EXPECTED_SIGNAL SIGALRM
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
@@ -559,3 +559,160 @@ main (int argc, char *argv[])
#endif
}
}
+
+/* The following functionality is only available if <pthread.h> was
+ included before this file. */
+#ifdef _PTHREAD_H
+
+/* Call pthread_sigmask with error checking. */
+static void
+xpthread_sigmask (int how, const sigset_t *set, sigset_t *oldset)
+{
+ if (pthread_sigmask (how, set, oldset) != 0)
+ {
+ write_message ("error: pthread_setmask failed\n");
+ _exit (1);
+ }
+}
+
+/* Call pthread_mutex_lock with error checking. */
+__attribute__ ((unused))
+static void
+xpthread_mutex_lock (pthread_mutex_t *mutex)
+{
+ int ret = pthread_mutex_lock (mutex);
+ if (ret != 0)
+ {
+ errno = ret;
+ printf ("error: pthread_mutex_lock: %m\n");
+ exit (1);
+ }
+}
+
+/* Call pthread_spin_lock with error checking. */
+__attribute__ ((unused))
+static void
+xpthread_spin_lock (pthread_spinlock_t *lock)
+{
+ int ret = pthread_spin_lock (lock);
+ if (ret != 0)
+ {
+ errno = ret;
+ printf ("error: pthread_spin_lock: %m\n");
+ exit (1);
+ }
+}
+
+/* Call pthread_cond_wait with error checking. */
+__attribute__ ((unused))
+static void
+xpthread_cond_wait (pthread_cond_t * cond,
+ pthread_mutex_t * mutex)
+{
+ int ret = pthread_cond_wait (cond, mutex);
+ if (ret != 0)
+ {
+ errno = ret;
+ printf ("error: pthread_cond_wait: %m\n");
+ exit (1);
+ }
+}
+
+/* Call pthread_barrier_wait with error checking. */
+__attribute__ ((unused))
+static int
+xpthread_barrier_wait (pthread_barrier_t *barrier)
+{
+ int ret = pthread_barrier_wait (barrier);
+ if (ret != 0 && ret != PTHREAD_BARRIER_SERIAL_THREAD)
+ {
+ errno = ret;
+ printf ("error: pthread_barrier_wait: %m\n");
+ exit (1);
+ }
+ return ret;
+}
+
+/* Call pthread_create with error checking. */
+static pthread_t
+xpthread_create (pthread_attr_t *attr,
+ void *(*thread_func) (void *), void *closure)
+{
+ pthread_t thr;
+ int ret = pthread_create (&thr, attr, thread_func, closure);
+ if (ret != 0)
+ {
+ errno = ret;
+ printf ("error: pthread_create: %m\n");
+ exit (1);
+ }
+ return thr;
+}
+
+/* Call pthread_detach with error checking. */
+static void
+xpthread_detach (pthread_t thr)
+{
+ int ret = pthread_detach (thr);
+ if (ret != 0)
+ {
+ errno = ret;
+ printf ("error: pthread_detach: %m\n");
+ exit (1);
+ }
+}
+
+/* Call pthread_join with error checking. */
+__attribute__ ((unused))
+static void *
+xpthread_join (pthread_t thr)
+{
+ void *result;
+ int ret = pthread_join (thr, &result);
+ if (ret != 0)
+ {
+ errno = ret;
+ printf ("error: pthread_join: %m\n");
+ exit (1);
+ }
+ return result;
+}
+
+/* Used to implement the delayed_exit function defined below. */
+static void *
+delayed_exit_thread (void *seconds_as_ptr)
+{
+ int seconds = (uintptr_t) seconds_as_ptr;
+ struct timespec delay = { seconds, 0 };
+ struct timespec remaining = {};
+ if (nanosleep (&delay, &remaining) != 0)
+ {
+ printf ("error: nanosleep: %m\n");
+ _exit (1);
+ }
+ /* Exit the process sucessfully. */
+ exit (0);
+ return NULL;
+}
+
+/* Exit (with status 0) after SECONDS have elapsed, from a helper
+ thread. The process is terminated with the exit function, so
+ atexit handlers are executed. */
+__attribute__ ((unused))
+static void
+delayed_exit (int seconds)
+{
+ /* Create the new thread with all signals blocked. */
+ sigset_t all_blocked;
+ sigfillset (&all_blocked);
+ sigset_t old_set;
+ xpthread_sigmask (SIG_SETMASK, &all_blocked, &old_set);
+ /* Create a detached thread. */
+ pthread_t thr = xpthread_create
+ (NULL, delayed_exit_thread, (void *) (uintptr_t) seconds);
+ xpthread_detach (thr);
+ /* Restore the original signal mask. */
+ xpthread_sigmask (SIG_SETMASK, &old_set, NULL);
+}
+
+#endif /* _PTHREAD_H */