[PATCH] malloc-standard: synchronize on fork

Rich Felker dalias at aerifal.cx
Sun Mar 27 19:21:41 UTC 2011


On Sat, Mar 26, 2011 at 08:28:04PM +0200, Timo Teräs wrote:
> Otherwise other threads can leave malloc state locked, and the child
> will hang indefinitely if it tries to malloc something.
> 
> Signed-off-by: Timo Teräs <timo.teras at iki.fi>
> ---
>  libc/stdlib/malloc-standard/free.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/libc/stdlib/malloc-standard/free.c b/libc/stdlib/malloc-standard/free.c
> index 39e54d6..df512cc 100644
> --- a/libc/stdlib/malloc-standard/free.c
> +++ b/libc/stdlib/malloc-standard/free.c
> @@ -118,6 +118,21 @@ int malloc_trim(size_t pad)
>    to inline it at all call points, which turns out not to be an
>    optimization at all. (Inlining it in __malloc_consolidate is fine though.)
>  */
> +static void _malloc_lock(void)
> +{
> +    __UCLIBC_MUTEX_LOCK_CANCEL_UNSAFE(__malloc_lock);
> +}
> +
> +static void _malloc_unlock(void)
> +{
> +    __UCLIBC_MUTEX_UNLOCK_CANCEL_UNSAFE(__malloc_lock);
> +}
> +
> +static void _malloc_reset(void)
> +{
> +    __UCLIBC_MUTEX_INIT_VAR(__malloc_lock);
> +}
> +
>  static void malloc_init_state(mstate av)
>  {
>      int     i;
> @@ -145,6 +160,8 @@ static void malloc_init_state(mstate av)
>  
>      av->top            = initial_top(av);
>      av->pagesize       = malloc_getpagesize;
> +
> +    __libc_atfork(_malloc_lock, _malloc_unlock, _malloc_reset);
>  }

This change by itself breaks as much as it fixes, unless the mutex
implementation is reentrant and recursive. fork() is specified to be
async-signal-safe, which means it's legal for a signal handler to call
fork() while malloc() is being executed. With your patch, this will
hang the program.

I ran into the same issue developing musl's implementation of malloc,
and for now I have left it alone. POSIX 2008 (SUSv4) went back on some
of the promises of previous versions, and has now declared that it's
UB to call any async-signal-unsafe functions (such as malloc()) after
fork() in a multi-threaded program, so your patch is not needed to
conform to the current standard, only to previous versions (and even
then it's not clear that the older standard required malloc() after
fork() to work in a threaded program).

I may later revisit this issue and "fix" it, but the fix is not simply
using locks. I would either need to ignore locking and actively repair
all the potentially-corrupted malloc structures in the child process,
or selectively determine which locks are held by the current thread
(due to having interrupted malloc() with a signal handler) and only
attempt to lock the other locks before forking. For uClibc which only
has one big malloc() lock, it may be easier, but I think you'll at
least need to fix the lock to be reentrant and either recursive or
error-checking. And that of course has a cost...

Rich



More information about the uClibc mailing list