[BZ,#16573] Fix for hang when using mtrace with MALLOC_TRACE set
Commit Message
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.
Kwok
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.
---
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. */
Comments
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. */
@@ -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);