From 6c4de62c59ba95420ee712e340c8dd8d0ab4b1f9 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Mon, 4 Oct 2010 11:35:48 -0700 Subject: [PATCH] libc - Fix some recursion issues during thread teardown * thread destructors called by sophisticated pthreaded programs such as firefox can wind up allocating/freeing space multiple times AFTER nmalloc's destructor is run. This can leave the mtmagazine in a weird state because it's destructor fails to NULL-out tp->mags[i].loaded and tp->mags[i].prev. * Properly NULL out tp->mags[i].{loaded,prev} in the destructor. * Lockout mtmagazine use while the mtmagazine is being initialized or destroyed. * Permanently lockout mtmagazine use after its destructor has been run. * Instead of trying to initialize the mtmagazine on the first free, which might not occur until the destrutor is run (causing pthreads to complain about destructors being left hanging), have libthread_xu call a new function _nmalloc_thr_init() during thread creation and do all the slab initialization there. --- lib/libc/include/libc_private.h | 1 + lib/libc/stdlib/nmalloc.c | 96 +++++++++++++++++++++++++++--------- lib/libthread_xu/thread/thr_create.c | 1 + 3 files changed, 75 insertions(+), 23 deletions(-) diff --git a/lib/libc/include/libc_private.h b/lib/libc/include/libc_private.h index 7b95fa47cd..e358b82e20 100644 --- a/lib/libc/include/libc_private.h +++ b/lib/libc/include/libc_private.h @@ -86,5 +86,6 @@ extern void (*__cleanup)(void); /* execve() with PATH processing to implement posix_spawnp() */ int _execvpe(const char *, char * const *, char * const *); +void _nmalloc_thr_init(void); #endif /* _LIBC_PRIVATE_H_ */ diff --git a/lib/libc/stdlib/nmalloc.c b/lib/libc/stdlib/nmalloc.c index c4bdd201c8..829a4cbead 100644 --- a/lib/libc/stdlib/nmalloc.c +++ b/lib/libc/stdlib/nmalloc.c @@ -194,7 +194,6 @@ typedef struct slglobaldata { #define SLZF_UNOTZEROD 0x0001 -#define MAG_NORECURSE 0x01 #define FASTSLABREALLOC 0x02 /* @@ -303,7 +302,7 @@ typedef struct thr_mags { * this variable when the code is compiled -fPIC */ #define TLS_ATTRIBUTE __attribute__ ((tls_model ("initial-exec"))); -static int mtmagazine_free_live = 0; +static int mtmagazine_free_live; static __thread thr_mags thread_mags TLS_ATTRIBUTE; static pthread_key_t thread_mags_key; static pthread_once_t thread_mags_once = PTHREAD_ONCE_INIT; @@ -416,6 +415,34 @@ malloc_init(void) } /* + * We have to install a handler for nmalloc thread teardowns when + * the thread is created. We cannot delay this because destructors in + * sophisticated userland programs can call malloc() for the first time + * during their thread exit. + * + * This routine is called directly from pthreads. + */ +void +_nmalloc_thr_init(void) +{ + thr_mags *tp; + + /* + * Disallow mtmagazine operations until the mtmagazine is + * initialized. + */ + tp = &thread_mags; + tp->init = -1; + + pthread_setspecific(thread_mags_key, tp); + if (mtmagazine_free_live == 0) { + mtmagazine_free_live = 1; + pthread_once(&thread_mags_once, mtmagazine_init); + } + tp->init = 1; +} + +/* * Thread locks. */ static __inline void @@ -1094,7 +1121,6 @@ _slabrealloc(void *ptr, size_t size) * checking memory limits in malloc. * * flags: - * MAG_NORECURSE Skip magazine layer * FASTSLABREALLOC Fast call from realloc * MPSAFE */ @@ -1124,12 +1150,6 @@ _slabfree(void *ptr, int flags, bigalloc_t *rbigp) if (ptr == ZERO_LENGTH_PTR) return; - /* Ensure that a destructor is in-place for thread-exit */ - if (mtmagazine_free_live == 0) { - mtmagazine_free_live = 1; - pthread_once(&thread_mags_once, &mtmagazine_init); - } - /* * Handle oversized allocations. */ @@ -1168,8 +1188,7 @@ fastslabrealloc: if (g_malloc_flags & SAFLAG_ZERO) bzero(ptr, size); - if (((flags & MAG_NORECURSE) == 0) && - (mtmagazine_free(zi, ptr) == 0)) + if (mtmagazine_free(zi, ptr) == 0) return; pgno = ((char *)ptr - (char *)z) >> PAGE_SHIFT; @@ -1326,8 +1345,17 @@ mtmagazine_alloc(int zi) magazine_depot *d; void *obj = NULL; + /* + * Do not try to access per-thread magazines while the mtmagazine + * is being initialized or destroyed. + */ tp = &thread_mags; + if (tp->init < 0) + return(NULL); + /* + * Primary per-thread allocation loop + */ for (;;) { /* If the loaded magazine has rounds, allocate and return */ if (((mp = tp->mags[zi].loaded) != NULL) && @@ -1378,13 +1406,17 @@ mtmagazine_free(int zi, void *ptr) magazine_depot *d; int rc = -1; + /* + * Do not try to access per-thread magazines while the mtmagazine + * is being initialized or destroyed. + */ tp = &thread_mags; + if (tp->init < 0) + return(-1); - if (tp->init == 0) { - pthread_setspecific(thread_mags_key, tp); - tp->init = 1; - } - + /* + * Primary per-thread freeing loop + */ for (;;) { /* If the loaded magazine has space, free directly to it */ if (((mp = tp->mags[zi].loaded) != NULL) && @@ -1441,13 +1473,18 @@ mtmagazine_free(int zi, void *ptr) } static void -mtmagazine_init(void) { - int i = 0; - i = pthread_key_create(&thread_mags_key,&mtmagazine_destructor); - if (i != 0) +mtmagazine_init(void) +{ + int error; + + error = pthread_key_create(&thread_mags_key, mtmagazine_destructor); + if (error) abort(); } +/* + * This function is only used by the thread exit destructor + */ static void mtmagazine_drain(struct magazine *mp) { @@ -1455,7 +1492,7 @@ mtmagazine_drain(struct magazine *mp) while (MAGAZINE_NOTEMPTY(mp)) { obj = magazine_alloc(mp, NULL); - _slabfree(obj, MAG_NORECURSE, NULL); + _slabfree(obj, 0, NULL); } } @@ -1464,6 +1501,10 @@ mtmagazine_drain(struct magazine *mp) * * When a thread exits, we reclaim all its resources; all its magazines are * drained and the structures are freed. + * + * WARNING! The destructor can be called multiple times if the larger user + * program has its own destructors which run after ours which + * allocate or free memory. */ static void mtmagazine_destructor(void *thrp) @@ -1472,16 +1513,25 @@ mtmagazine_destructor(void *thrp) struct magazine *mp; int i; + /* + * Prevent further use of mtmagazines while we are destructing + * them, as well as for any destructors which are run after us + * prior to the thread actually being destroyed. + */ + tp->init = -1; + for (i = 0; i < NZONES; i++) { mp = tp->mags[i].loaded; + tp->mags[i].loaded = NULL; if (mp != NULL && MAGAZINE_NOTEMPTY(mp)) mtmagazine_drain(mp); - _slabfree(mp, MAG_NORECURSE, NULL); + _slabfree(mp, 0, NULL); mp = tp->mags[i].prev; + tp->mags[i].prev = NULL; if (mp != NULL && MAGAZINE_NOTEMPTY(mp)) mtmagazine_drain(mp); - _slabfree(mp, MAG_NORECURSE, NULL); + _slabfree(mp, 0, NULL); } } diff --git a/lib/libthread_xu/thread/thr_create.c b/lib/libthread_xu/thread/thr_create.c index 0da32d8606..04e108eb5e 100644 --- a/lib/libthread_xu/thread/thr_create.c +++ b/lib/libthread_xu/thread/thr_create.c @@ -236,6 +236,7 @@ thread_start(void *arg) if (curthread->flags & THR_FLAGS_NEED_SUSPEND) _thr_suspend_check(curthread); + _nmalloc_thr_init(); /* Run the current thread's start routine with argument: */ _pthread_exit(curthread->start_routine(curthread->arg)); -- 2.11.4.GIT