|
 |
8a8cfb |
commit 669ff911e2571f74a2668493e326ac9a505776bd
|
|
|
8a8cfb |
Author: Florian Weimer <fweimer@redhat.com>
|
|
|
8a8cfb |
Date: Fri Feb 8 12:46:19 2019 +0100
|
|
|
8a8cfb |
|
|
|
8a8cfb |
nptl: Avoid fork handler lock for async-signal-safe fork [BZ #24161]
|
|
|
8a8cfb |
|
|
|
8a8cfb |
Commit 27761a1042daf01987e7d79636d0c41511c6df3c ("Refactor atfork
|
|
|
8a8cfb |
handlers") introduced a lock, atfork_lock, around fork handler list
|
|
|
8a8cfb |
accesses. It turns out that this lock occasionally results in
|
|
|
8a8cfb |
self-deadlocks in malloc/tst-mallocfork2:
|
|
|
8a8cfb |
|
|
|
8a8cfb |
(gdb) bt
|
|
|
8a8cfb |
#0 __lll_lock_wait_private ()
|
|
|
8a8cfb |
at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:63
|
|
|
8a8cfb |
#1 0x00007f160c6f927a in __run_fork_handlers (who=(unknown: 209394016),
|
|
|
8a8cfb |
who@entry=atfork_run_prepare) at register-atfork.c:116
|
|
|
8a8cfb |
#2 0x00007f160c6b7897 in __libc_fork () at ../sysdeps/nptl/fork.c:58
|
|
|
8a8cfb |
#3 0x00000000004027d6 in sigusr1_handler (signo=<optimized out>)
|
|
|
8a8cfb |
at tst-mallocfork2.c:80
|
|
|
8a8cfb |
#4 sigusr1_handler (signo=<optimized out>) at tst-mallocfork2.c:64
|
|
|
8a8cfb |
#5 <signal handler called>
|
|
|
8a8cfb |
#6 0x00007f160c6f92e4 in __run_fork_handlers (who=who@entry=atfork_run_parent)
|
|
|
8a8cfb |
at register-atfork.c:136
|
|
|
8a8cfb |
#7 0x00007f160c6b79a2 in __libc_fork () at ../sysdeps/nptl/fork.c:152
|
|
|
8a8cfb |
#8 0x0000000000402567 in do_test () at tst-mallocfork2.c:156
|
|
|
8a8cfb |
#9 0x0000000000402dd2 in support_test_main (argc=1, argv=0x7ffc81ef1ab0,
|
|
|
8a8cfb |
config=config@entry=0x7ffc81ef1970) at support_test_main.c:350
|
|
|
8a8cfb |
#10 0x0000000000402362 in main (argc=<optimized out>, argv=<optimized out>)
|
|
|
8a8cfb |
at ../support/test-driver.c:168
|
|
|
8a8cfb |
|
|
|
8a8cfb |
If no locking happens in the single-threaded case (where fork is
|
|
|
8a8cfb |
expected to be async-signal-safe), this deadlock is avoided.
|
|
|
8a8cfb |
(pthread_atfork is not required to be async-signal-safe, so a fork
|
|
|
8a8cfb |
call from a signal handler interrupting pthread_atfork is not
|
|
|
8a8cfb |
a problem.)
|
|
|
8a8cfb |
|
|
|
8a8cfb |
diff --git a/nptl/register-atfork.c b/nptl/register-atfork.c
|
|
|
8a8cfb |
index bc797b7..80a1bec 100644
|
|
|
8a8cfb |
--- a/nptl/register-atfork.c
|
|
|
8a8cfb |
+++ b/nptl/register-atfork.c
|
|
|
8a8cfb |
@@ -107,13 +107,14 @@ __unregister_atfork (void *dso_handle)
|
|
|
8a8cfb |
}
|
|
|
8a8cfb |
|
|
|
8a8cfb |
void
|
|
|
8a8cfb |
-__run_fork_handlers (enum __run_fork_handler_type who)
|
|
|
8a8cfb |
+__run_fork_handlers (enum __run_fork_handler_type who, _Bool do_locking)
|
|
|
8a8cfb |
{
|
|
|
8a8cfb |
struct fork_handler *runp;
|
|
|
8a8cfb |
|
|
|
8a8cfb |
if (who == atfork_run_prepare)
|
|
|
8a8cfb |
{
|
|
|
8a8cfb |
- lll_lock (atfork_lock, LLL_PRIVATE);
|
|
|
8a8cfb |
+ if (do_locking)
|
|
|
8a8cfb |
+ lll_lock (atfork_lock, LLL_PRIVATE);
|
|
|
8a8cfb |
size_t sl = fork_handler_list_size (&fork_handlers);
|
|
|
8a8cfb |
for (size_t i = sl; i > 0; i--)
|
|
|
8a8cfb |
{
|
|
|
8a8cfb |
@@ -133,7 +134,8 @@ __run_fork_handlers (enum __run_fork_handler_type who)
|
|
|
8a8cfb |
else if (who == atfork_run_parent && runp->parent_handler)
|
|
|
8a8cfb |
runp->parent_handler ();
|
|
|
8a8cfb |
}
|
|
|
8a8cfb |
- lll_unlock (atfork_lock, LLL_PRIVATE);
|
|
|
8a8cfb |
+ if (do_locking)
|
|
|
8a8cfb |
+ lll_unlock (atfork_lock, LLL_PRIVATE);
|
|
|
8a8cfb |
}
|
|
|
8a8cfb |
}
|
|
|
8a8cfb |
|
|
|
8a8cfb |
diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
|
|
|
8a8cfb |
index bd68f18..14b69a6 100644
|
|
|
8a8cfb |
--- a/sysdeps/nptl/fork.c
|
|
|
8a8cfb |
+++ b/sysdeps/nptl/fork.c
|
|
|
8a8cfb |
@@ -55,7 +55,7 @@ __libc_fork (void)
|
|
|
8a8cfb |
but our current fork implementation is not. */
|
|
|
8a8cfb |
bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads);
|
|
|
8a8cfb |
|
|
|
8a8cfb |
- __run_fork_handlers (atfork_run_prepare);
|
|
|
8a8cfb |
+ __run_fork_handlers (atfork_run_prepare, multiple_threads);
|
|
|
8a8cfb |
|
|
|
8a8cfb |
/* If we are not running multiple threads, we do not have to
|
|
|
8a8cfb |
preserve lock state. If fork runs from a signal handler, only
|
|
|
8a8cfb |
@@ -134,7 +134,7 @@ __libc_fork (void)
|
|
|
8a8cfb |
__rtld_lock_initialize (GL(dl_load_lock));
|
|
|
8a8cfb |
|
|
|
8a8cfb |
/* Run the handlers registered for the child. */
|
|
|
8a8cfb |
- __run_fork_handlers (atfork_run_child);
|
|
|
8a8cfb |
+ __run_fork_handlers (atfork_run_child, multiple_threads);
|
|
|
8a8cfb |
}
|
|
|
8a8cfb |
else
|
|
|
8a8cfb |
{
|
|
|
8a8cfb |
@@ -149,7 +149,7 @@ __libc_fork (void)
|
|
|
8a8cfb |
}
|
|
|
8a8cfb |
|
|
|
8a8cfb |
/* Run the handlers registered for the parent. */
|
|
|
8a8cfb |
- __run_fork_handlers (atfork_run_parent);
|
|
|
8a8cfb |
+ __run_fork_handlers (atfork_run_parent, multiple_threads);
|
|
|
8a8cfb |
}
|
|
|
8a8cfb |
|
|
|
8a8cfb |
return pid;
|
|
|
8a8cfb |
diff --git a/sysdeps/nptl/fork.h b/sysdeps/nptl/fork.h
|
|
|
8a8cfb |
index a1c3b26..99ed760 100644
|
|
|
8a8cfb |
--- a/sysdeps/nptl/fork.h
|
|
|
8a8cfb |
+++ b/sysdeps/nptl/fork.h
|
|
|
8a8cfb |
@@ -52,9 +52,11 @@ enum __run_fork_handler_type
|
|
|
8a8cfb |
- atfork_run_child: run all the CHILD_HANDLER and unlocks the internal
|
|
|
8a8cfb |
lock.
|
|
|
8a8cfb |
- atfork_run_parent: run all the PARENT_HANDLER and unlocks the internal
|
|
|
8a8cfb |
- lock. */
|
|
|
8a8cfb |
-extern void __run_fork_handlers (enum __run_fork_handler_type who)
|
|
|
8a8cfb |
- attribute_hidden;
|
|
|
8a8cfb |
+ lock.
|
|
|
8a8cfb |
+
|
|
|
8a8cfb |
+ Perform locking only if DO_LOCKING. */
|
|
|
8a8cfb |
+extern void __run_fork_handlers (enum __run_fork_handler_type who,
|
|
|
8a8cfb |
+ _Bool do_locking) attribute_hidden;
|
|
|
8a8cfb |
|
|
|
8a8cfb |
/* C library side function to register new fork handlers. */
|
|
|
8a8cfb |
extern int __register_atfork (void (*__prepare) (void),
|