On 07/12/22(Wed) 22:17, Joel Knight wrote:
> Hi. As first mentioned on misc[1], I've identified a deadlock in libc
> when a process forks, the children are multi-threaded, and they set
> one or more atfork callbacks. The diff below causes ATFORK_UNLOCK() to
> release the lock even when the process isn't multi-threaded. This
> avoids the deadlock. With this patch applied, the test case I have for
> this issue succeeds and there are no new failures during a full 'make
> regress'.
>
> Threading is outside my area of expertise so I've no idea if the fix
> proposed here is appropriate. I'm happy to take or test feedback.
>
> The diff is below and a clean copy is here:
> https://www.packetmischief.ca/files/patches/atfork_on_fork.diff.
This sounds legit to me. Anyone some time wants to take a look?
> .joel
>
>
> [1] https://marc.info/?l=openbsd-misc&m=166926508819790&w=2
>
>
>
> Index: lib/libc/include/thread_private.h
> ===
> RCS file: /data/cvs-mirror/OpenBSD/src/lib/libc/include/thread_private.h,v
> retrieving revision 1.36
> diff -p -u -r1.36 thread_private.h
> --- lib/libc/include/thread_private.h 6 Jan 2021 19:54:17 - 1.36
> +++ lib/libc/include/thread_private.h 8 Dec 2022 04:28:45 -
> @@ -228,7 +228,7 @@ __END_HIDDEN_DECLS
> } while (0)
> #define _ATFORK_UNLOCK() \
> do { \
> - if (__isthreaded) \
> + if (_thread_cb.tc_atfork_unlock != NULL) \
> _thread_cb.tc_atfork_unlock(); \
> } while (0)
>
> Index: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
> ===
> RCS file: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
> diff -N regress/lib/libpthread/pthread_atfork_on_fork/Makefile
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/lib/libpthread/pthread_atfork_on_fork/Makefile 7 Dec 2022
> 04:38:39 -
> @@ -0,0 +1,9 @@
> +# $OpenBSD$
> +
> +PROG= pthread_atfork_on_fork
> +
> +REGRESS_TARGETS= timeout
> +timeout:
> + timeout 10s ./${PROG}
> +
> +.include
> Index: regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> ===
> RCS file:
> regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> diff -N regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> 7 Dec 2022 04:59:10 -
> @@ -0,0 +1,94 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2022 Joel Knight
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +/*
> + * This test exercises atfork lock/unlock through multiple generations of
> + * forked child processes where each child also becomes multi-threaded.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#define NUMBER_OF_GENERATIONS 4
> +#define NUMBER_OF_TEST_THREADS 2
> +
> +#define SAY(...) do { \
> +fprintf(stderr, "pid %5i ", getpid()); \
> +fprintf(stderr, __VA_ARGS__); \
> +} while (0)
> +
> +void
> +prepare(void)
> +{
> +/* Do nothing */
> +}
> +
> +void *
> +thread(void *arg)
> +{
> +return (NULL);
> +}
> +
> +int
> +test(int fork_level)
> +{
> +pid_t proc_pid;
> +size_t thread_index;
> +pthread_t threads[NUMBER_OF_TEST_THREADS];
> +
> +proc_pid = fork();
> +fork_level = fork_level - 1;
> +
> +if (proc_pid == 0) {
> +SAY("generation %i\n", fork_level);
> +pthread_atfork(prepare, NULL, NULL);
> +
> +for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
> thread_index++) {
> +pthread_create(&threads[thread_index], NULL, thread, NULL);
> +}
> +
> +for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
> thread_index++) {
> +pthread_join(threads[thread_index], NULL);
> +}
> +
> +if (fork_level > 0) {
> +test(fork_level);
> +}
> +
> +SAY("exiting\n");
> +exit(0);
> +}
> +else {
> +SAY("parent waiting on child %i\n", proc_pid);
> +waitpid(proc_pid, 0, 0);
> +}
> +
> +return (0);
> +}
> +
> +int
> +main(int argc, char *argv[])
>
