| commit 3abf3bd4edc86fb28c099cc85203cb46a811e0b8 |
| Author: Florian Weimer <fweimer@redhat.com> |
| Date: Mon Sep 13 11:06:08 2021 +0200 |
| |
| nptl: pthread_kill, pthread_cancel should not fail after exit (bug 19193) |
| |
| This closes one remaining race condition related to bug 12889: if |
| the thread already exited on the kernel side, returning ESRCH |
| is not correct because that error is reserved for the thread IDs |
| (pthread_t values) whose lifetime has ended. In case of a |
| kernel-side exit and a valid thread ID, no signal needs to be sent |
| and cancellation does not have an effect, so just return 0. |
| |
| sysdeps/pthread/tst-kill4.c triggers undefined behavior and is |
| removed with this commit. |
| |
| Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> |
| (cherry picked from commit 8af8456004edbab71f8903a60a3cae442cf6fe69) |
| |
| diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c |
| index cc25ff21f364e8a4..9bac6e3b76a20312 100644 |
| |
| |
| @@ -62,10 +62,11 @@ __pthread_cancel (pthread_t th) |
| { |
| volatile struct pthread *pd = (volatile struct pthread *) th; |
| |
| - /* Make sure the descriptor is valid. */ |
| - if (INVALID_TD_P (pd)) |
| - /* Not a valid thread handle. */ |
| - return ESRCH; |
| + if (pd->tid == 0) |
| + /* The thread has already exited on the kernel side. Its outcome |
| + (regular exit, other cancelation) has already been |
| + determined. */ |
| + return 0; |
| |
| static int init_sigcancel = 0; |
| if (atomic_load_relaxed (&init_sigcancel) == 0) |
| diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c |
| index f79a2b26fc7f72e5..5d4c86f9205a6fb5 100644 |
| |
| |
| @@ -46,7 +46,12 @@ __pthread_kill_internal (pthread_t threadid, int signo) |
| ? INTERNAL_SYSCALL_ERRNO (val) : 0); |
| } |
| else |
| - val = ESRCH; |
| + /* The kernel reports that the thread has exited. POSIX specifies |
| + the ESRCH error only for the case when the lifetime of a thread |
| + ID has ended, but calling pthread_kill on such a thread ID is |
| + undefined in glibc. Therefore, do not treat kernel thread exit |
| + as an error. */ |
| + val = 0; |
| |
| return val; |
| } |
| diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile |
| index 42f9fc507263657d..dedfa0d290da4949 100644 |
| |
| |
| @@ -89,7 +89,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ |
| tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \ |
| tst-join14 tst-join15 \ |
| tst-key1 tst-key2 tst-key3 tst-key4 \ |
| - tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \ |
| + tst-kill1 tst-kill2 tst-kill3 tst-kill5 tst-kill6 \ |
| tst-locale1 tst-locale2 \ |
| tst-memstream \ |
| tst-mutex-errorcheck tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 \ |
| @@ -118,6 +118,9 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ |
| tst-unload \ |
| tst-unwind-thread \ |
| tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \ |
| + tst-pthread_cancel-exited \ |
| + tst-pthread_kill-exited \ |
| + # tests |
| |
| tests-time64 := \ |
| tst-abstime-time64 \ |
| diff --git a/sysdeps/pthread/tst-kill4.c b/sysdeps/pthread/tst-kill4.c |
| deleted file mode 100644 |
| index 9563939792b96ebd..0000000000000000 |
| |
| |
| @@ -1,90 +0,0 @@ |
| -/* Copyright (C) 2003-2021 Free Software Foundation, Inc. |
| - This file is part of the GNU C Library. |
| - Contributed by Ulrich Drepper <drepper@redhat.com>, 2003. |
| - |
| - 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 |
| - <https://www.gnu.org/licenses/>. */ |
| - |
| -#include <errno.h> |
| -#include <pthread.h> |
| -#include <signal.h> |
| -#include <stdio.h> |
| -#include <stdlib.h> |
| -#include <unistd.h> |
| - |
| - |
| -static void * |
| -tf (void *a) |
| -{ |
| - return NULL; |
| -} |
| - |
| - |
| -int |
| -do_test (void) |
| -{ |
| - pthread_attr_t at; |
| - if (pthread_attr_init (&at) != 0) |
| - { |
| - puts ("attr_create failed"); |
| - exit (1); |
| - } |
| - |
| - /* Limit thread stack size, because if it is too large, pthread_join |
| - will free it immediately rather than put it into stack cache. */ |
| - if (pthread_attr_setstacksize (&at, 2 * 1024 * 1024) != 0) |
| - { |
| - puts ("setstacksize failed"); |
| - exit (1); |
| - } |
| - |
| - pthread_t th; |
| - if (pthread_create (&th, &at, tf, NULL) != 0) |
| - { |
| - puts ("create failed"); |
| - exit (1); |
| - } |
| - |
| - pthread_attr_destroy (&at); |
| - |
| - if (pthread_join (th, NULL) != 0) |
| - { |
| - puts ("join failed"); |
| - exit (1); |
| - } |
| - |
| - /* The following only works because we assume here something about |
| - the implementation. Namely, that the memory allocated for the |
| - thread descriptor is not going away, that the TID field is |
| - cleared and therefore the signal is sent to process 0, and that |
| - we can savely assume there is no other process with this ID at |
| - that time. */ |
| - int e = pthread_kill (th, 0); |
| - if (e == 0) |
| - { |
| - puts ("pthread_kill succeeded"); |
| - exit (1); |
| - } |
| - if (e != ESRCH) |
| - { |
| - puts ("pthread_kill didn't return ESRCH"); |
| - exit (1); |
| - } |
| - |
| - return 0; |
| -} |
| - |
| - |
| -#define TEST_FUNCTION do_test () |
| -#include "../test-skeleton.c" |
| diff --git a/sysdeps/pthread/tst-pthread_cancel-exited.c b/sysdeps/pthread/tst-pthread_cancel-exited.c |
| new file mode 100644 |
| index 0000000000000000..811c9bee07ab2638 |
| |
| |
| @@ -0,0 +1,45 @@ |
| +/* Test that pthread_kill succeeds for an exited thread. |
| + Copyright (C) 2021 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 |
| + <https://www.gnu.org/licenses/>. */ |
| + |
| +/* This test verifies that pthread_kill returns 0 (and not ESRCH) for |
| + a thread that has exited on the kernel side. */ |
| + |
| +#include <stddef.h> |
| +#include <support/support.h> |
| +#include <support/xthread.h> |
| + |
| +static void * |
| +noop_thread (void *closure) |
| +{ |
| + return NULL; |
| +} |
| + |
| +static int |
| +do_test (void) |
| +{ |
| + pthread_t thr = xpthread_create (NULL, noop_thread, NULL); |
| + |
| + support_wait_for_thread_exit (); |
| + |
| + xpthread_cancel (thr); |
| + xpthread_join (thr); |
| + |
| + return 0; |
| +} |
| + |
| +#include <support/test-driver.c> |
| diff --git a/sysdeps/pthread/tst-pthread_kill-exited.c b/sysdeps/pthread/tst-pthread_kill-exited.c |
| new file mode 100644 |
| index 0000000000000000..7575fb6d58cae99c |
| |
| |
| @@ -0,0 +1,46 @@ |
| +/* Test that pthread_kill succeeds for an exited thread. |
| + Copyright (C) 2021 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 |
| + <https://www.gnu.org/licenses/>. */ |
| + |
| +/* This test verifies that pthread_kill returns 0 (and not ESRCH) for |
| + a thread that has exited on the kernel side. */ |
| + |
| +#include <signal.h> |
| +#include <stddef.h> |
| +#include <support/support.h> |
| +#include <support/xthread.h> |
| + |
| +static void * |
| +noop_thread (void *closure) |
| +{ |
| + return NULL; |
| +} |
| + |
| +static int |
| +do_test (void) |
| +{ |
| + pthread_t thr = xpthread_create (NULL, noop_thread, NULL); |
| + |
| + support_wait_for_thread_exit (); |
| + |
| + xpthread_kill (thr, SIGUSR1); |
| + xpthread_join (thr); |
| + |
| + return 0; |
| +} |
| + |
| +#include <support/test-driver.c> |