* [PATCH 2/2] posix: Correctly block/unblock all signals on Linux posix_spawn
2016-09-14 18:56 [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn Adhemerval Zanella
@ 2016-09-14 18:56 ` Adhemerval Zanella
2016-09-20 18:41 ` Adhemerval Zanella
2016-09-20 20:03 ` Florian Weimer
2016-09-20 18:17 ` [PATCH 1/2] posix: Correctly enable/disable cancellation " Adhemerval Zanella
2016-09-20 19:24 ` Florian Weimer
2 siblings, 2 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2016-09-14 18:56 UTC (permalink / raw)
To: libc-alpha
This patch correctly block and unblocks all signals when executing
Linux posix_spawn by using the __libc_signal_{un}block_all functions
instead of default sigprocmask. The latter might remove both
SIGCANCEL and SIGSETXID from the blocked signal list.
Checked on x86_64, i686, powerpc64le, and aarch64.
* sysdeps/unix/sysv/linux/spawni.c (__spawnix): Correctly block and unblock
all signals when executing the clone vfork child.
(SIGALL_SET): Remove macro.
---
ChangeLog | 4 ++++
sysdeps/unix/sysv/linux/spawni.c | 8 ++------
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index f7cb96f..c34f8fd 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -60,10 +60,6 @@
normal program exit with the exit code 127. */
#define SPAWN_ERROR 127
-/* We need to block both SIGCANCEL and SIGSETXID. */
-#define SIGALL_SET \
- ((__sigset_t) { .__val = {[0 ... _SIGSET_NWORDS-1 ] = -1 } })
-
#ifdef __ia64__
# define CLONE(__fn, __stack, __stacksize, __flags, __args) \
__clone2 (__fn, __stack, __stacksize, __flags, __args, 0, 0, 0)
@@ -331,7 +327,7 @@ __spawnix (pid_t * pid, const char *file,
args.xflags = xflags;
args.err = 0;
- __sigprocmask (SIG_BLOCK, &SIGALL_SET, &args.oldmask);
+ __libc_signal_block_all (&args.oldmask);
/* The clone flags used will create a new child that will run in the same
memory space (CLONE_VM) and the execution of calling thread will be
@@ -357,7 +353,7 @@ __spawnix (pid_t * pid, const char *file,
if ((ec == 0) && (pid != NULL))
*pid = new_pid;
- __sigprocmask (SIG_SETMASK, &args.oldmask, 0);
+ __libc_signal_restore_set (&args.oldmask);
__libc_ptf_call (__pthread_setcancelstate, (state, NULL), 0);
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn
@ 2016-09-14 18:56 Adhemerval Zanella
2016-09-14 18:56 ` [PATCH 2/2] posix: Correctly block/unblock all signals " Adhemerval Zanella
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2016-09-14 18:56 UTC (permalink / raw)
To: libc-alpha
This patch correctly enable and disable asynchronous cancellation on
Linux posix_spawn. Current code invert the logic by enabling and
disabling instead. A new test that checks if posix_spawn is not in fact
a cancellation entrypoint is added.
Checked on x86_64, i686, powerpc64le, and aarch64.
* nptl/Makefile (tests): Add tst-exec5.
* nptl/tst-exec5.c: New file.
* sysdeps/unix/sysv/linux/spawni.c (__spawni): Correctly enable and disable
asynchronous cancellation.
---
nptl/Makefile | 2 +-
nptl/tst-exec5.c | 200 +++++++++++++++++++++++++++++++++++++++
sysdeps/unix/sysv/linux/spawni.c | 6 +-
4 files changed, 212 insertions(+), 3 deletions(-)
create mode 100644 nptl/tst-exec5.c
diff --git a/nptl/Makefile b/nptl/Makefile
index 2ddcd2b..390e1f7 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -268,7 +268,7 @@ tests = tst-typesizes \
tst-flock1 tst-flock2 \
tst-signal1 tst-signal2 tst-signal3 tst-signal4 tst-signal5 \
tst-signal6 tst-signal7 \
- tst-exec1 tst-exec2 tst-exec3 tst-exec4 \
+ tst-exec1 tst-exec2 tst-exec3 tst-exec4 tst-exec5 \
tst-exit1 tst-exit2 tst-exit3 \
tst-stdio1 tst-stdio2 \
tst-stack1 tst-stack2 tst-stack3 tst-stack4 tst-pthread-getattr \
diff --git a/nptl/tst-exec5.c b/nptl/tst-exec5.c
new file mode 100644
index 0000000..5527b23
--- /dev/null
+++ b/nptl/tst-exec5.c
@@ -0,0 +1,200 @@
+/* Check if posix_spawn does not act as a cancellation entrypoint.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <paths.h>
+#include <pthread.h>
+#include <signal.h>
+#include <spawn.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/wait.h>
+
+static pthread_barrier_t b;
+
+static struct thread_args
+{
+ pid_t pid;
+ int fd[2];
+} targ;
+
+static void *
+tf (void *arg)
+{
+ int e = pthread_barrier_wait (&b);
+ if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
+ {
+ puts ("error: pthread_barrier_wait failed");
+ exit (1);
+ }
+
+ posix_spawn_file_actions_t a;
+ if (posix_spawn_file_actions_init (&a) != 0)
+ {
+ puts ("error: spawn_file_actions_init failed");
+ exit (1);
+ }
+
+ if (posix_spawn_file_actions_adddup2 (&a, targ.fd[1], STDOUT_FILENO) != 0)
+ {
+ puts ("error: spawn_file_actions_adddup2 failed");
+ exit (1);
+ }
+
+ if (posix_spawn_file_actions_addclose (&a, targ.fd[0]) != 0)
+ {
+ puts ("error: spawn_file_actions_addclose");
+ exit (1);
+ }
+
+ char *argv[] = { (char *) _PATH_BSHELL, (char *) "-c", (char *) "echo $$",
+ NULL };
+ if (posix_spawn (&targ.pid, _PATH_BSHELL, &a, NULL, argv, NULL) != 0)
+ {
+ puts ("error: spawn failed");
+ exit (1);
+ }
+
+ return NULL;
+}
+
+
+static int
+do_test (void)
+{
+ /* The test basically pipes a 'echo $$' created by a thread with a
+ cancellation pending. It then checks if the thread is not
+ cancelled, the process is created, and if the output is the expected
+ one (the process pid). */
+
+ if (pipe (targ.fd) != 0)
+ {
+ puts ("error: pipe failed");
+ exit (1);
+ }
+
+ /* Not interested in knowing when the pipe is closed. */
+ if (sigignore (SIGPIPE) != 0)
+ {
+ puts ("error: sigignore failed");
+ exit (1);
+ }
+
+ if (pthread_barrier_init (&b, NULL, 2) != 0)
+ {
+ puts ("error: pthread_barrier_init failed");
+ exit (1);
+ }
+
+ pthread_t th;
+ if (pthread_create (&th, NULL, tf, &targ) != 0)
+ {
+ puts ("error: pthread_create failed");
+ exit (1);
+ }
+
+ if (pthread_cancel (th) != 0)
+ {
+ puts ("error: pthread_cancel failed");
+ return 1;
+ }
+
+ int e = pthread_barrier_wait (&b);
+ if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
+ {
+ puts ("error: pthread_barrier_wait failed");
+ exit (1);
+ }
+
+ void *r;
+ if (pthread_join (th, &r) != 0)
+ {
+ puts ("error: pthread_join failed");
+ exit (1);
+ }
+
+ if (r == PTHREAD_CANCELED)
+ {
+ puts ("error: thread cancelled");
+ exit (1);
+ }
+
+ close (targ.fd[1]);
+
+ /* The spawn_posix pid should be set by thread, check if it was executed
+ correctly and with expected output. */
+
+ char buf[64];
+ ssize_t n;
+ bool seen_pid = false;
+ while (TEMP_FAILURE_RETRY ((n = read (targ.fd[0], buf, sizeof (buf)))) > 0)
+ {
+ /* We only expect to read the PID. */
+ char *endp;
+ long int rpid = strtol (buf, &endp, 10);
+
+ if (*endp != '\n')
+ {
+ printf ("error: didn't parse whole line: \"%s\"\n", buf);
+ exit (1);
+ }
+ if (endp == buf)
+ {
+ puts ("error: read empty line");
+ exit (1);
+ }
+
+ if (rpid != targ.pid)
+ {
+ printf ("error: found \"%s\", expected PID %ld\n", buf,
+ (long int) targ.pid);
+ exit (1);
+ }
+
+ if (seen_pid)
+ {
+ puts ("error: found more than one PID line");
+ exit (1);
+ }
+
+ seen_pid = true;
+ }
+
+ close (targ.fd[0]);
+
+ int status;
+ int err = waitpid (targ.pid, &status, 0);
+ if (err != targ.pid)
+ {
+ puts ("errnor: waitpid failed");
+ exit (1);
+ }
+
+ if (!seen_pid)
+ {
+ puts ("error: didn't get PID");
+ exit (1);
+ }
+
+ return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include <test-skeleton.c>
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index 0b76657..f7cb96f 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -317,7 +317,9 @@ __spawnix (pid_t * pid, const char *file,
return errno;
/* Disable asynchronous cancellation. */
- int cs = LIBC_CANCEL_ASYNC ();
+ int state;
+ __libc_ptf_call (__pthread_setcancelstate,
+ (PTHREAD_CANCEL_DISABLE, &state), 0);
args.file = file;
args.exec = exec;
@@ -357,7 +359,7 @@ __spawnix (pid_t * pid, const char *file,
__sigprocmask (SIG_SETMASK, &args.oldmask, 0);
- LIBC_CANCEL_RESET (cs);
+ __libc_ptf_call (__pthread_setcancelstate, (state, NULL), 0);
return ec;
}
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn
2016-09-14 18:56 [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn Adhemerval Zanella
2016-09-14 18:56 ` [PATCH 2/2] posix: Correctly block/unblock all signals " Adhemerval Zanella
@ 2016-09-20 18:17 ` Adhemerval Zanella
2016-09-20 19:24 ` Florian Weimer
2 siblings, 0 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2016-09-20 18:17 UTC (permalink / raw)
To: libc-alpha
If no one opposes I will commit it shortly.
On 14/09/2016 15:56, Adhemerval Zanella wrote:
> This patch correctly enable and disable asynchronous cancellation on
> Linux posix_spawn. Current code invert the logic by enabling and
> disabling instead. A new test that checks if posix_spawn is not in fact
> a cancellation entrypoint is added.
>
> Checked on x86_64, i686, powerpc64le, and aarch64.
>
> * nptl/Makefile (tests): Add tst-exec5.
> * nptl/tst-exec5.c: New file.
> * sysdeps/unix/sysv/linux/spawni.c (__spawni): Correctly enable and disable
> asynchronous cancellation.
> ---
> nptl/Makefile | 2 +-
> nptl/tst-exec5.c | 200 +++++++++++++++++++++++++++++++++++++++
> sysdeps/unix/sysv/linux/spawni.c | 6 +-
> 4 files changed, 212 insertions(+), 3 deletions(-)
> create mode 100644 nptl/tst-exec5.c
>
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 2ddcd2b..390e1f7 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -268,7 +268,7 @@ tests = tst-typesizes \
> tst-flock1 tst-flock2 \
> tst-signal1 tst-signal2 tst-signal3 tst-signal4 tst-signal5 \
> tst-signal6 tst-signal7 \
> - tst-exec1 tst-exec2 tst-exec3 tst-exec4 \
> + tst-exec1 tst-exec2 tst-exec3 tst-exec4 tst-exec5 \
> tst-exit1 tst-exit2 tst-exit3 \
> tst-stdio1 tst-stdio2 \
> tst-stack1 tst-stack2 tst-stack3 tst-stack4 tst-pthread-getattr \
> diff --git a/nptl/tst-exec5.c b/nptl/tst-exec5.c
> new file mode 100644
> index 0000000..5527b23
> --- /dev/null
> +++ b/nptl/tst-exec5.c
> @@ -0,0 +1,200 @@
> +/* Check if posix_spawn does not act as a cancellation entrypoint.
> + Copyright (C) 2016 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <paths.h>
> +#include <pthread.h>
> +#include <signal.h>
> +#include <spawn.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/wait.h>
> +
> +static pthread_barrier_t b;
> +
> +static struct thread_args
> +{
> + pid_t pid;
> + int fd[2];
> +} targ;
> +
> +static void *
> +tf (void *arg)
> +{
> + int e = pthread_barrier_wait (&b);
> + if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
> + {
> + puts ("error: pthread_barrier_wait failed");
> + exit (1);
> + }
> +
> + posix_spawn_file_actions_t a;
> + if (posix_spawn_file_actions_init (&a) != 0)
> + {
> + puts ("error: spawn_file_actions_init failed");
> + exit (1);
> + }
> +
> + if (posix_spawn_file_actions_adddup2 (&a, targ.fd[1], STDOUT_FILENO) != 0)
> + {
> + puts ("error: spawn_file_actions_adddup2 failed");
> + exit (1);
> + }
> +
> + if (posix_spawn_file_actions_addclose (&a, targ.fd[0]) != 0)
> + {
> + puts ("error: spawn_file_actions_addclose");
> + exit (1);
> + }
> +
> + char *argv[] = { (char *) _PATH_BSHELL, (char *) "-c", (char *) "echo $$",
> + NULL };
> + if (posix_spawn (&targ.pid, _PATH_BSHELL, &a, NULL, argv, NULL) != 0)
> + {
> + puts ("error: spawn failed");
> + exit (1);
> + }
> +
> + return NULL;
> +}
> +
> +
> +static int
> +do_test (void)
> +{
> + /* The test basically pipes a 'echo $$' created by a thread with a
> + cancellation pending. It then checks if the thread is not
> + cancelled, the process is created, and if the output is the expected
> + one (the process pid). */
> +
> + if (pipe (targ.fd) != 0)
> + {
> + puts ("error: pipe failed");
> + exit (1);
> + }
> +
> + /* Not interested in knowing when the pipe is closed. */
> + if (sigignore (SIGPIPE) != 0)
> + {
> + puts ("error: sigignore failed");
> + exit (1);
> + }
> +
> + if (pthread_barrier_init (&b, NULL, 2) != 0)
> + {
> + puts ("error: pthread_barrier_init failed");
> + exit (1);
> + }
> +
> + pthread_t th;
> + if (pthread_create (&th, NULL, tf, &targ) != 0)
> + {
> + puts ("error: pthread_create failed");
> + exit (1);
> + }
> +
> + if (pthread_cancel (th) != 0)
> + {
> + puts ("error: pthread_cancel failed");
> + return 1;
> + }
> +
> + int e = pthread_barrier_wait (&b);
> + if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
> + {
> + puts ("error: pthread_barrier_wait failed");
> + exit (1);
> + }
> +
> + void *r;
> + if (pthread_join (th, &r) != 0)
> + {
> + puts ("error: pthread_join failed");
> + exit (1);
> + }
> +
> + if (r == PTHREAD_CANCELED)
> + {
> + puts ("error: thread cancelled");
> + exit (1);
> + }
> +
> + close (targ.fd[1]);
> +
> + /* The spawn_posix pid should be set by thread, check if it was executed
> + correctly and with expected output. */
> +
> + char buf[64];
> + ssize_t n;
> + bool seen_pid = false;
> + while (TEMP_FAILURE_RETRY ((n = read (targ.fd[0], buf, sizeof (buf)))) > 0)
> + {
> + /* We only expect to read the PID. */
> + char *endp;
> + long int rpid = strtol (buf, &endp, 10);
> +
> + if (*endp != '\n')
> + {
> + printf ("error: didn't parse whole line: \"%s\"\n", buf);
> + exit (1);
> + }
> + if (endp == buf)
> + {
> + puts ("error: read empty line");
> + exit (1);
> + }
> +
> + if (rpid != targ.pid)
> + {
> + printf ("error: found \"%s\", expected PID %ld\n", buf,
> + (long int) targ.pid);
> + exit (1);
> + }
> +
> + if (seen_pid)
> + {
> + puts ("error: found more than one PID line");
> + exit (1);
> + }
> +
> + seen_pid = true;
> + }
> +
> + close (targ.fd[0]);
> +
> + int status;
> + int err = waitpid (targ.pid, &status, 0);
> + if (err != targ.pid)
> + {
> + puts ("errnor: waitpid failed");
> + exit (1);
> + }
> +
> + if (!seen_pid)
> + {
> + puts ("error: didn't get PID");
> + exit (1);
> + }
> +
> + return 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include <test-skeleton.c>
> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
> index 0b76657..f7cb96f 100644
> --- a/sysdeps/unix/sysv/linux/spawni.c
> +++ b/sysdeps/unix/sysv/linux/spawni.c
> @@ -317,7 +317,9 @@ __spawnix (pid_t * pid, const char *file,
> return errno;
>
> /* Disable asynchronous cancellation. */
> - int cs = LIBC_CANCEL_ASYNC ();
> + int state;
> + __libc_ptf_call (__pthread_setcancelstate,
> + (PTHREAD_CANCEL_DISABLE, &state), 0);
>
> args.file = file;
> args.exec = exec;
> @@ -357,7 +359,7 @@ __spawnix (pid_t * pid, const char *file,
>
> __sigprocmask (SIG_SETMASK, &args.oldmask, 0);
>
> - LIBC_CANCEL_RESET (cs);
> + __libc_ptf_call (__pthread_setcancelstate, (state, NULL), 0);
>
> return ec;
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] posix: Correctly block/unblock all signals on Linux posix_spawn
2016-09-14 18:56 ` [PATCH 2/2] posix: Correctly block/unblock all signals " Adhemerval Zanella
@ 2016-09-20 18:41 ` Adhemerval Zanella
2016-09-20 20:03 ` Florian Weimer
1 sibling, 0 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2016-09-20 18:41 UTC (permalink / raw)
To: libc-alpha
If no one opposes I will commit it shortly.
On 14/09/2016 15:56, Adhemerval Zanella wrote:
> This patch correctly block and unblocks all signals when executing
> Linux posix_spawn by using the __libc_signal_{un}block_all functions
> instead of default sigprocmask. The latter might remove both
> SIGCANCEL and SIGSETXID from the blocked signal list.
>
> Checked on x86_64, i686, powerpc64le, and aarch64.
>
> * sysdeps/unix/sysv/linux/spawni.c (__spawnix): Correctly block and unblock
> all signals when executing the clone vfork child.
> (SIGALL_SET): Remove macro.
> ---
> ChangeLog | 4 ++++
> sysdeps/unix/sysv/linux/spawni.c | 8 ++------
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
> index f7cb96f..c34f8fd 100644
> --- a/sysdeps/unix/sysv/linux/spawni.c
> +++ b/sysdeps/unix/sysv/linux/spawni.c
> @@ -60,10 +60,6 @@
> normal program exit with the exit code 127. */
> #define SPAWN_ERROR 127
>
> -/* We need to block both SIGCANCEL and SIGSETXID. */
> -#define SIGALL_SET \
> - ((__sigset_t) { .__val = {[0 ... _SIGSET_NWORDS-1 ] = -1 } })
> -
> #ifdef __ia64__
> # define CLONE(__fn, __stack, __stacksize, __flags, __args) \
> __clone2 (__fn, __stack, __stacksize, __flags, __args, 0, 0, 0)
> @@ -331,7 +327,7 @@ __spawnix (pid_t * pid, const char *file,
> args.xflags = xflags;
> args.err = 0;
>
> - __sigprocmask (SIG_BLOCK, &SIGALL_SET, &args.oldmask);
> + __libc_signal_block_all (&args.oldmask);
>
> /* The clone flags used will create a new child that will run in the same
> memory space (CLONE_VM) and the execution of calling thread will be
> @@ -357,7 +353,7 @@ __spawnix (pid_t * pid, const char *file,
> if ((ec == 0) && (pid != NULL))
> *pid = new_pid;
>
> - __sigprocmask (SIG_SETMASK, &args.oldmask, 0);
> + __libc_signal_restore_set (&args.oldmask);
>
> __libc_ptf_call (__pthread_setcancelstate, (state, NULL), 0);
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn
2016-09-14 18:56 [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn Adhemerval Zanella
2016-09-14 18:56 ` [PATCH 2/2] posix: Correctly block/unblock all signals " Adhemerval Zanella
2016-09-20 18:17 ` [PATCH 1/2] posix: Correctly enable/disable cancellation " Adhemerval Zanella
@ 2016-09-20 19:24 ` Florian Weimer
2016-09-20 19:54 ` Adhemerval Zanella
2 siblings, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2016-09-20 19:24 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
* Adhemerval Zanella:
> + int e = pthread_barrier_wait (&b);
> + if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
> + {
> + puts ("error: pthread_barrier_wait failed");
> + exit (1);
> + }
We have xpthread_barrier_wait now (also below).
> + /* The test basically pipes a 'echo $$' created by a thread with a
> + cancellation pending. It then checks if the thread is not
> + cancelled, the process is created, and if the output is the expected
> + one (the process pid). */
Don't you have to try this multiple times, to see if you can hit the
race?
> +
> + if (pipe (targ.fd) != 0)
> + {
> + puts ("error: pipe failed");
> + exit (1);
> + }
> +
> + /* Not interested in knowing when the pipe is closed. */
> + if (sigignore (SIGPIPE) != 0)
> + {
> + puts ("error: sigignore failed");
> + exit (1);
> + }
> +
> + if (pthread_barrier_init (&b, NULL, 2) != 0)
> + {
> + puts ("error: pthread_barrier_init failed");
> + exit (1);
> + }
> +
> + pthread_t th;
> + if (pthread_create (&th, NULL, tf, &targ) != 0)
xpthread_create.
> + if (pthread_join (th, &r) != 0)
xpthread_join.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn
2016-09-20 19:24 ` Florian Weimer
@ 2016-09-20 19:54 ` Adhemerval Zanella
2016-09-20 19:58 ` Florian Weimer
0 siblings, 1 reply; 8+ messages in thread
From: Adhemerval Zanella @ 2016-09-20 19:54 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 20/09/2016 16:24, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> + int e = pthread_barrier_wait (&b);
>> + if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
>> + {
>> + puts ("error: pthread_barrier_wait failed");
>> + exit (1);
>> + }
>
> We have xpthread_barrier_wait now (also below).
>
Ack, I will change to xpthread_barrier_wait.
>> + /* The test basically pipes a 'echo $$' created by a thread with a
>> + cancellation pending. It then checks if the thread is not
>> + cancelled, the process is created, and if the output is the expected
>> + one (the process pid). */
>
> Don't you have to try this multiple times, to see if you can hit the
> race?
Not in this test because the idea is to check if posix_spawn acts or not
on a pending cancellation, not if an asynchronous cancellation cancels the
thread while the thread executes posix_spawn.
>
>> +
>> + if (pipe (targ.fd) != 0)
>> + {
>> + puts ("error: pipe failed");
>> + exit (1);
>> + }
>> +
>> + /* Not interested in knowing when the pipe is closed. */
>> + if (sigignore (SIGPIPE) != 0)
>> + {
>> + puts ("error: sigignore failed");
>> + exit (1);
>> + }
>> +
>> + if (pthread_barrier_init (&b, NULL, 2) != 0)
>> + {
>> + puts ("error: pthread_barrier_init failed");
>> + exit (1);
>> + }
>> +
>> + pthread_t th;
>> + if (pthread_create (&th, NULL, tf, &targ) != 0)
>
> xpthread_create.
Ack.
>
>> + if (pthread_join (th, &r) != 0)
>
> xpthread_join.
>
Ack.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn
2016-09-20 19:54 ` Adhemerval Zanella
@ 2016-09-20 19:58 ` Florian Weimer
0 siblings, 0 replies; 8+ messages in thread
From: Florian Weimer @ 2016-09-20 19:58 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
* Adhemerval Zanella:
>> Don't you have to try this multiple times, to see if you can hit the
>> race?
>
> Not in this test because the idea is to check if posix_spawn acts or not
> on a pending cancellation, not if an asynchronous cancellation cancels the
> thread while the thread executes posix_spawn.
Ah, I'm still trying to wrap my head around the different forms of
cancellation. Patch is good as-is then. Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] posix: Correctly block/unblock all signals on Linux posix_spawn
2016-09-14 18:56 ` [PATCH 2/2] posix: Correctly block/unblock all signals " Adhemerval Zanella
2016-09-20 18:41 ` Adhemerval Zanella
@ 2016-09-20 20:03 ` Florian Weimer
1 sibling, 0 replies; 8+ messages in thread
From: Florian Weimer @ 2016-09-20 20:03 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
* Adhemerval Zanella:
> This patch correctly block and unblocks all signals when executing
> Linux posix_spawn by using the __libc_signal_{un}block_all functions
> instead of default sigprocmask. The latter might remove both
> SIGCANCEL and SIGSETXID from the blocked signal list.
>
> Checked on x86_64, i686, powerpc64le, and aarch64.
>
> * sysdeps/unix/sysv/linux/spawni.c (__spawnix): Correctly block and unblock
> all signals when executing the clone vfork child.
> (SIGALL_SET): Remove macro.
Looks good to me. Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-20 20:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14 18:56 [PATCH 1/2] posix: Correctly enable/disable cancellation on Linux posix_spawn Adhemerval Zanella
2016-09-14 18:56 ` [PATCH 2/2] posix: Correctly block/unblock all signals " Adhemerval Zanella
2016-09-20 18:41 ` Adhemerval Zanella
2016-09-20 20:03 ` Florian Weimer
2016-09-20 18:17 ` [PATCH 1/2] posix: Correctly enable/disable cancellation " Adhemerval Zanella
2016-09-20 19:24 ` Florian Weimer
2016-09-20 19:54 ` Adhemerval Zanella
2016-09-20 19:58 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).