Re: [PATCH][BZ #16573] Fix for hang when using mtrace with MALLOC_TRACE set

[Carlos O'Donell <codonell at redhat dot com>]

On 8/8/14 3:46 PM, Kwok Cheung Yeung wrote:
> A hang can occur if the mtrace output is diverted to a file (by
> setting the MALLOC_TRACE environment variable) and an mtrace problem
> is found. The mtrace hook leads to a call of __libc_message(), which
> eventually results in a call to malloc(), causing another mtrace hook
> to be called.  This hook then attempts to reacquire the same lock
> that was already claimed by the first hook, resulting in an infinite
> hang.
>
> This patch fixes this by making all mtrace hooks temporarily
> uninstall all mtrace hooks for the duration of the call, so that
> there will not be another call to an mtrace hook further down the
> call chain.  

I'm cleaning up the queue of old malloc patches.

I'm proposing a similar patch but refactored slightly.

Thank you for waiting almost 5 years :}

> 2014-08-08  Kwok Cheung Yeung  <kcy@codesourcery.com>
>
>      [BZ #16573]
>      * malloc/mtrace.c (tr_freehook): Move function prototype to
>      before the hook definitions.  Disable all hooks before calling
>      the caller or old hook, and re-enable all hooks afterwards.
>      (tr_mallochook): Likewise.
>      (tr_reallochook): Likewise.
>      (tr_freehook): Likewise.
> ---
> diff --git a/malloc/mtrace.c b/malloc/mtrace.c
> index 91e5710..593b212 100644
> --- a/malloc/mtrace.c
> +++ b/malloc/mtrace.c
> @@ -120,6 +120,10 @@ lock_and_info (const __ptr_t caller, Dl_info *mem)
>     return res;
>   }
>
> +static __ptr_t tr_mallochook (size_t, const __ptr_t);
> +static __ptr_t tr_reallochook (__ptr_t, size_t, const __ptr_t);
> +static __ptr_t tr_memalignhook (size_t, size_t, const __ptr_t);
> +
>   static void
>   tr_freehook (__ptr_t ptr, const __ptr_t caller)
>   {
> @@ -138,11 +142,17 @@ tr_freehook (__ptr_t ptr, const __ptr_t caller)
>         __libc_lock_lock (lock);
>       }
>     __free_hook = tr_old_free_hook;
> +  __malloc_hook = tr_old_malloc_hook;
> +  __realloc_hook = tr_old_realloc_hook;
> +  __memalign_hook = tr_old_memalign_hook;
>     if (tr_old_free_hook != NULL)
>       (*tr_old_free_hook)(ptr, caller);
>     else
>       free (ptr);
>     __free_hook = tr_freehook;
> +  __malloc_hook = tr_mallochook;
> +  __realloc_hook = tr_reallochook;
> +  __memalign_hook = tr_memalignhook;
>     __libc_lock_unlock (lock);
>   }
>
> @@ -154,12 +164,18 @@ tr_mallochook (size_t size, const __ptr_t caller)
>     Dl_info mem;
>     Dl_info *info = lock_and_info (caller, &mem);
>
> +  __free_hook = tr_old_free_hook;
>     __malloc_hook = tr_old_malloc_hook;
> +  __realloc_hook = tr_old_realloc_hook;
> +  __memalign_hook = tr_old_memalign_hook;
>     if (tr_old_malloc_hook != NULL)
>       hdr = (__ptr_t) (*tr_old_malloc_hook)(size, caller);
>     else
>       hdr = (__ptr_t) malloc (size);
> +  __free_hook = tr_freehook;
>     __malloc_hook = tr_mallochook;
> +  __realloc_hook = tr_reallochook;
> +  __memalign_hook = tr_memalignhook;
>
>     tr_where (caller, info);
>     /* We could be printing a NULL here; that's OK.  */
> @@ -187,6 +203,7 @@ tr_reallochook (__ptr_t ptr, size_t size, const __ptr_t caller)
>     __free_hook = tr_old_free_hook;
>     __malloc_hook = tr_old_malloc_hook;
>     __realloc_hook = tr_old_realloc_hook;
> +  __memalign_hook = tr_old_memalign_hook;
>     if (tr_old_realloc_hook != NULL)
>       hdr = (__ptr_t) (*tr_old_realloc_hook)(ptr, size, caller);
>     else
> @@ -194,6 +211,7 @@ tr_reallochook (__ptr_t ptr, size_t size, const __ptr_t caller)
>     __free_hook = tr_freehook;
>     __malloc_hook = tr_mallochook;
>     __realloc_hook = tr_reallochook;
> +  __memalign_hook = tr_memalignhook;
>
>     tr_where (caller, info);
>     if (hdr == NULL)
> @@ -229,14 +247,18 @@ tr_memalignhook (size_t alignment, size_t size, const __ptr_t caller)
>     Dl_info mem;
>     Dl_info *info = lock_and_info (caller, &mem);
>
> -  __memalign_hook = tr_old_memalign_hook;
> +  __free_hook = tr_old_free_hook;
>     __malloc_hook = tr_old_malloc_hook;
> +  __realloc_hook = tr_old_realloc_hook;
> +  __memalign_hook = tr_old_memalign_hook;
>     if (tr_old_memalign_hook != NULL)
>       hdr = (__ptr_t) (*tr_old_memalign_hook)(alignment, size, caller);
>     else
>       hdr = (__ptr_t) memalign (alignment, size);
> -  __memalign_hook = tr_memalignhook;
> +  __free_hook = tr_freehook;
>     __malloc_hook = tr_mallochook;
> +  __realloc_hook = tr_reallochook;
> +  __memalign_hook = tr_memalignhook;
>
>     tr_where (caller, info);
>     /* We could be printing a NULL here; that's OK.  */


--
Cheers,
Carlos.