@@ -, +, @@ Revert "Refactor atfork handlers" This reverts commit 27761a1042daf01987e7d79636d0c41511c6df3c. --- a/ChangeLog +++ a/ChangeLog @@ -6777,22 +6777,6 @@ * sysdeps/sparc/fpu/libm-test-ulps: Update. - * nptl/Makefile (routines): Remove unregister-atfork. - * nptl/register-atfork.c (fork_handler_pool): Remove variable. - (fork_handler_alloc): Remove function. - (fork_handlers, fork_handler_init): New variables. - (__fork_lock): Rename to atfork_lock. - (__register_atfork, __unregister_atfork, libc_freeres_fn): Rewrite - to use a dynamic array to add/remove atfork handlers. - * sysdeps/nptl/fork.c (__libc_fork): Likewise. - * sysdeps/nptl/fork.h (__fork_lock, __fork_handlers, __linkin_atfork): - Remove declaration. - (fork_handler): Remove next, refcntr, and need_signal member. - (__run_fork_handler_type): New enum. - (__run_fork_handlers): New prototype. - * nptl/register-atfork.c: Remove file. - * sysdeps/nptl/libc-lockP.h (__libc_atfork): Remove declaration. - * sysdeps/nptl/nptl-signals.h: Move to ... * sysdeps/generic/internal-signals.h: ... here. Adjust internal comments. --- a/nptl/Makefile +++ a/nptl/Makefile @@ -29,8 +29,8 @@ extra-libs-others := $(extra-libs) routines = alloca_cutoff forward libc-lowlevellock libc-cancellation \ libc-cleanup libc_pthread_init libc_multiple_threads \ - register-atfork pthread_atfork pthread_self thrd_current \ - thrd_equal thrd_sleep thrd_yield + register-atfork unregister-atfork pthread_atfork pthread_self \ + thrd_current thrd_equal thrd_sleep thrd_yield shared-only-routines = forward static-only-routines = pthread_atfork --- a/nptl/register-atfork.c +++ a/nptl/register-atfork.c @@ -22,127 +22,123 @@ #include #include -#define DYNARRAY_ELEMENT struct fork_handler -#define DYNARRAY_STRUCT fork_handler_list -#define DYNARRAY_PREFIX fork_handler_list_ -#define DYNARRAY_INITIAL_SIZE 48 -#include -static struct fork_handler_list fork_handlers; -static bool fork_handler_init = false; +struct fork_handler *__fork_handlers; + +/* Lock to protect allocation and deallocation of fork handlers. */ +int __fork_lock = LLL_LOCK_INITIALIZER; + + +/* Number of pre-allocated handler entries. */ +#define NHANDLER 48 + +/* Memory pool for fork handler structures. */ +static struct fork_handler_pool +{ + struct fork_handler_pool *next; + struct fork_handler mem[NHANDLER]; +} fork_handler_pool; + + +static struct fork_handler * +fork_handler_alloc (void) +{ + struct fork_handler_pool *runp = &fork_handler_pool; + struct fork_handler *result = NULL; + unsigned int i; + + do + { + /* Search for an empty entry. */ + for (i = 0; i < NHANDLER; ++i) + if (runp->mem[i].refcntr == 0) + goto found; + } + while ((runp = runp->next) != NULL); + + /* We have to allocate a new entry. */ + runp = (struct fork_handler_pool *) calloc (1, sizeof (*runp)); + if (runp != NULL) + { + /* Enqueue the new memory pool into the list. */ + runp->next = fork_handler_pool.next; + fork_handler_pool.next = runp; + + /* We use the last entry on the page. This means when we start + searching from the front the next time we will find the first + entry unused. */ + i = NHANDLER - 1; + + found: + result = &runp->mem[i]; + result->refcntr = 1; + result->need_signal = 0; + } + + return result; +} -static int atfork_lock = LLL_LOCK_INITIALIZER; int __register_atfork (void (*prepare) (void), void (*parent) (void), void (*child) (void), void *dso_handle) { - lll_lock (atfork_lock, LLL_PRIVATE); + /* Get the lock to not conflict with other allocations. */ + lll_lock (__fork_lock, LLL_PRIVATE); - if (!fork_handler_init) - { - fork_handler_list_init (&fork_handlers); - fork_handler_init = true; - } + struct fork_handler *newp = fork_handler_alloc (); - struct fork_handler *newp = fork_handler_list_emplace (&fork_handlers); if (newp != NULL) { + /* Initialize the new record. */ newp->prepare_handler = prepare; newp->parent_handler = parent; newp->child_handler = child; newp->dso_handle = dso_handle; + + __linkin_atfork (newp); } /* Release the lock. */ - lll_unlock (atfork_lock, LLL_PRIVATE); + lll_unlock (__fork_lock, LLL_PRIVATE); return newp == NULL ? ENOMEM : 0; } libc_hidden_def (__register_atfork) -static struct fork_handler * -fork_handler_list_find (struct fork_handler_list *fork_handlers, - void *dso_handle) -{ - for (size_t i = 0; i < fork_handler_list_size (fork_handlers); i++) - { - struct fork_handler *elem = fork_handler_list_at (fork_handlers, i); - if (elem->dso_handle == dso_handle) - return elem; - } - return NULL; -} void -__unregister_atfork (void *dso_handle) +attribute_hidden +__linkin_atfork (struct fork_handler *newp) { - lll_lock (atfork_lock, LLL_PRIVATE); - - struct fork_handler *first = fork_handler_list_find (&fork_handlers, - dso_handle); - /* Removing is done by shifting the elements in the way the elements - that are not to be removed appear in the beginning in dynarray. - This avoid the quadradic run-time if a naive strategy to remove and - shift one element at time. */ - if (first != NULL) - { - struct fork_handler *new_end = first; - first++; - for (; first != fork_handler_list_end (&fork_handlers); ++first) - { - if (first->dso_handle != dso_handle) - { - *new_end = *first; - ++new_end; - } - } - - ptrdiff_t removed = first - new_end; - for (size_t i = 0; i < removed; i++) - fork_handler_list_remove_last (&fork_handlers); - } - - lll_unlock (atfork_lock, LLL_PRIVATE); + do + newp->next = __fork_handlers; + while (catomic_compare_and_exchange_bool_acq (&__fork_handlers, + newp, newp->next) != 0); } -void -__run_fork_handlers (enum __run_fork_handler_type who) + +libc_freeres_fn (free_mem) { - struct fork_handler *runp; + /* Get the lock to not conflict with running forks. */ + lll_lock (__fork_lock, LLL_PRIVATE); - if (who == atfork_run_prepare) - { - lll_lock (atfork_lock, LLL_PRIVATE); - size_t sl = fork_handler_list_size (&fork_handlers); - for (size_t i = sl; i > 0; i--) - { - runp = fork_handler_list_at (&fork_handlers, i - 1); - if (runp->prepare_handler != NULL) - runp->prepare_handler (); - } - } - else - { - size_t sl = fork_handler_list_size (&fork_handlers); - for (size_t i = 0; i < sl; i++) - { - runp = fork_handler_list_at (&fork_handlers, i); - if (who == atfork_run_child && runp->child_handler) - runp->child_handler (); - else if (who == atfork_run_parent && runp->parent_handler) - runp->parent_handler (); - } - lll_unlock (atfork_lock, LLL_PRIVATE); - } -} + /* No more fork handlers. */ + __fork_handlers = NULL; + /* Free eventually allocated memory blocks for the object pool. */ + struct fork_handler_pool *runp = fork_handler_pool.next; -libc_freeres_fn (free_mem) -{ - lll_lock (atfork_lock, LLL_PRIVATE); + memset (&fork_handler_pool, '\0', sizeof (fork_handler_pool)); - fork_handler_list_free (&fork_handlers); + /* Release the lock. */ + lll_unlock (__fork_lock, LLL_PRIVATE); - lll_unlock (atfork_lock, LLL_PRIVATE); + /* We can free the memory after releasing the lock. */ + while (runp != NULL) + { + struct fork_handler_pool *oldp = runp; + runp = runp->next; + free (oldp); + } } --- a/nptl/unregister-atfork.c +++ a/nptl/unregister-atfork.c @@ -0,0 +1,121 @@ +/* Copyright (C) 2002-2018 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Ulrich Drepper , 2002. + + 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 + . */ + +#include +#include +#include +#include +#include + + +void +__unregister_atfork (void *dso_handle) +{ + /* Check whether there is any entry in the list which we have to + remove. It is likely that this is not the case so don't bother + getting the lock. + + We do not worry about other threads adding entries for this DSO + right this moment. If this happens this is a race and we can do + whatever we please. The program will crash anyway seen. */ + struct fork_handler *runp = __fork_handlers; + struct fork_handler *lastp = NULL; + + while (runp != NULL) + if (runp->dso_handle == dso_handle) + break; + else + { + lastp = runp; + runp = runp->next; + } + + if (runp == NULL) + /* Nothing to do. */ + return; + + /* Get the lock to not conflict with additions or deletions. Note + that there couldn't have been another thread deleting something. + The __unregister_atfork function is only called from the + dlclose() code which itself serializes the operations. */ + lll_lock (__fork_lock, LLL_PRIVATE); + + /* We have to create a new list with all the entries we don't remove. */ + struct deleted_handler + { + struct fork_handler *handler; + struct deleted_handler *next; + } *deleted = NULL; + + /* Remove the entries for the DSO which is unloaded from the list. + It's a single linked list so readers are. */ + do + { + again: + if (runp->dso_handle == dso_handle) + { + if (lastp == NULL) + { + /* We have to use an atomic operation here because + __linkin_atfork also uses one. */ + if (catomic_compare_and_exchange_bool_acq (&__fork_handlers, + runp->next, runp) + != 0) + { + runp = __fork_handlers; + goto again; + } + } + else + lastp->next = runp->next; + + /* We cannot overwrite the ->next element now. Put the deleted + entries in a separate list. */ + struct deleted_handler *newp = alloca (sizeof (*newp)); + newp->handler = runp; + newp->next = deleted; + deleted = newp; + } + else + lastp = runp; + + runp = runp->next; + } + while (runp != NULL); + + /* Release the lock. */ + lll_unlock (__fork_lock, LLL_PRIVATE); + + /* Walk the list of all entries which have to be deleted. */ + while (deleted != NULL) + { + /* We need to be informed by possible current users. */ + deleted->handler->need_signal = 1; + /* Make sure this gets written out first. */ + atomic_write_barrier (); + + /* Decrement the reference counter. If it does not reach zero + wait for the last user. */ + atomic_decrement (&deleted->handler->refcntr); + unsigned int val; + while ((val = deleted->handler->refcntr) != 0) + futex_wait_simple (&deleted->handler->refcntr, val, FUTEX_PRIVATE); + + deleted = deleted->next; + } +} --- a/sysdeps/nptl/fork.c +++ a/sysdeps/nptl/fork.c @@ -48,6 +48,11 @@ pid_t __libc_fork (void) { pid_t pid; + struct used_handler + { + struct fork_handler *handler; + struct used_handler *next; + } *allp = NULL; /* Determine if we are running multiple threads. We skip some fork handlers in the single-thread case, to make fork safer to use in @@ -55,7 +60,60 @@ __libc_fork (void) but our current fork implementation is not. */ bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads); - __run_fork_handlers (atfork_run_prepare); + /* Run all the registered preparation handlers. In reverse order. + While doing this we build up a list of all the entries. */ + struct fork_handler *runp; + while ((runp = __fork_handlers) != NULL) + { + /* Make sure we read from the current RUNP pointer. */ + atomic_full_barrier (); + + unsigned int oldval = runp->refcntr; + + if (oldval == 0) + /* This means some other thread removed the list just after + the pointer has been loaded. Try again. Either the list + is empty or we can retry it. */ + continue; + + /* Bump the reference counter. */ + if (atomic_compare_and_exchange_bool_acq (&__fork_handlers->refcntr, + oldval + 1, oldval)) + /* The value changed, try again. */ + continue; + + /* We bumped the reference counter for the first entry in the + list. That means that none of the following entries will + just go away. The unloading code works in the order of the + list. + + While executing the registered handlers we are building a + list of all the entries so that we can go backward later on. */ + while (1) + { + /* Execute the handler if there is one. */ + if (runp->prepare_handler != NULL) + runp->prepare_handler (); + + /* Create a new element for the list. */ + struct used_handler *newp + = (struct used_handler *) alloca (sizeof (*newp)); + newp->handler = runp; + newp->next = allp; + allp = newp; + + /* Advance to the next handler. */ + runp = runp->next; + if (runp == NULL) + break; + + /* Bump the reference counter for the next entry. */ + atomic_increment (&runp->refcntr); + } + + /* We are done. */ + break; + } /* If we are not running multiple threads, we do not have to preserve lock state. If fork runs from a signal handler, only @@ -134,7 +192,29 @@ __libc_fork (void) __rtld_lock_initialize (GL(dl_load_lock)); /* Run the handlers registered for the child. */ - __run_fork_handlers (atfork_run_child); + while (allp != NULL) + { + if (allp->handler->child_handler != NULL) + allp->handler->child_handler (); + + /* Note that we do not have to wake any possible waiter. + This is the only thread in the new process. The count + may have been bumped up by other threads doing a fork. + We reset it to 1, to avoid waiting for non-existing + thread(s) to release the count. */ + allp->handler->refcntr = 1; + + /* XXX We could at this point look through the object pool + and mark all objects not on the __fork_handlers list as + unused. This is necessary in case the fork() happened + while another thread called dlclose() and that call had + to create a new list. */ + + allp = allp->next; + } + + /* Initialize the fork lock. */ + __fork_lock = LLL_LOCK_INITIALIZER; } else { @@ -149,7 +229,17 @@ __libc_fork (void) } /* Run the handlers registered for the parent. */ - __run_fork_handlers (atfork_run_parent); + while (allp != NULL) + { + if (allp->handler->parent_handler != NULL) + allp->handler->parent_handler (); + + if (atomic_decrement_and_test (&allp->handler->refcntr) + && allp->handler->need_signal) + futex_wake (&allp->handler->refcntr, 1, FUTEX_PRIVATE); + + allp = allp->next; + } } return pid; --- a/sysdeps/nptl/fork.h +++ a/sysdeps/nptl/fork.h @@ -24,37 +24,29 @@ extern unsigned long int __fork_generation attribute_hidden; /* Pointer to the fork generation counter in the thread library. */ extern unsigned long int *__fork_generation_pointer attribute_hidden; +/* Lock to protect allocation and deallocation of fork handlers. */ +extern int __fork_lock attribute_hidden; + /* Elements of the fork handler lists. */ struct fork_handler { + struct fork_handler *next; void (*prepare_handler) (void); void (*parent_handler) (void); void (*child_handler) (void); void *dso_handle; + unsigned int refcntr; + int need_signal; }; +/* The single linked list of all currently registered for handlers. */ +extern struct fork_handler *__fork_handlers attribute_hidden; + + /* Function to call to unregister fork handlers. */ extern void __unregister_atfork (void *dso_handle) attribute_hidden; #define UNREGISTER_ATFORK(dso_handle) __unregister_atfork (dso_handle) -enum __run_fork_handler_type -{ - atfork_run_prepare, - atfork_run_child, - atfork_run_parent -}; - -/* Run the atfork handlers and lock/unlock the internal lock depending - of the WHO argument: - - - atfork_run_prepare: run all the PREPARE_HANDLER in reverse order of - insertion and locks the internal lock. - - atfork_run_child: run all the CHILD_HANDLER and unlocks the internal - lock. - - atfork_run_parent: run all the PARENT_HANDLER and unlocks the internal - lock. */ -extern void __run_fork_handlers (enum __run_fork_handler_type who) - attribute_hidden; /* C library side function to register new fork handlers. */ extern int __register_atfork (void (*__prepare) (void), @@ -62,3 +54,6 @@ extern int __register_atfork (void (*__prepare) (void), void (*__child) (void), void *dso_handle); libc_hidden_proto (__register_atfork) + +/* Add a new element to the fork list. */ +extern void __linkin_atfork (struct fork_handler *newp) attribute_hidden; --- a/sysdeps/nptl/libc-lockP.h +++ a/sysdeps/nptl/libc-lockP.h @@ -319,6 +319,8 @@ __libc_cleanup_routine (struct __pthread_cleanup_frame *f) /* Register handlers to execute before and after `fork'. Note that the last parameter is NULL. The handlers registered by the libc are never removed so this is OK. */ +#define __libc_atfork(PREPARE, PARENT, CHILD) \ + __register_atfork (PREPARE, PARENT, CHILD, NULL) extern int __register_atfork (void (*__prepare) (void), void (*__parent) (void), void (*__child) (void),