arena cache being reused

[Jason Evans]

Integrated:

	https://github.com/jemalloc/jemalloc/commit/45e9f66c280e1ba8bebf7bed387a43bc9e45536d

I was able to trigger the bug and verify your fix by slightly modifying the tsd test.

Thanks,
Jason

> On Aug 17, 2015, at 3:57 PM, Christopher Ferris <cferris at google.com> wrote:
> 
> Sorry, I meant to follow up before the release, but I didn't get a chance.
> 
> Yes, it only repros on 4.0.0 since I think this is the first time you've used the arenas_cache. It's a bit tricky to trigger, since you need to allocate memory from a pthread_key_destroy function. I don't have a good test case since I wasn't sure exactly what triggered the path to create the arenas_cache again. It's also tricky since the memory that gets corrupted is something in a small bin in my configuration (2 arenas, I also artificially constrained the number of tcache entries). So it's likely that most code would never trigger this exact problem.
> 
> Christopher
> 
> On Mon, Aug 17, 2015 at 3:20 PM, Jason Evans <jasone at canonware.com> wrote:
> On Jul 8, 2015, at 3:42 PM, Christopher Ferris <cferris at google.com> wrote:
> > Using the current version of the dev jemalloc, I found a case where jemalloc reuses a previously freed pointer. Specifically, the arena cache pointer can get freed, but reused.
> >
> > This can happen when a thread is ending and the key destroy functions are being called. If the jemalloc key destroy function is called, the arena cache is destroyed. But if another key destroy function is called which allocates memory, the old arena cache pointer can be reused, and have the arena pointers written to it.
> >
> > I think the fix is to change the arenas_cache_cleanup function to:
> >
> > void
> > arenas_cache_cleanup(tsd_t *tsd)
> > {
> >         arena_t **arenas_cache;
> >
> >         arenas_cache = tsd_arenas_cache_get(tsd);
> >         if (arenas_cache != NULL) {
> >                 bool *arenas_cache_bypassp = tsd_arenas_cache_bypassp_get(tsd);
> >                 *arenas_cache_bypassp = true;
> >                 tsd_arenas_cache_set(tsd, NULL);
> >                 a0dalloc(arenas_cache);
> >         }
> > }
> >
> > I believe the bypass has to be set so that another arena cache is not allocated since that memory would be leaked since there is not going to be another call to the arenas_cache_cleanup function. I think this is the only possible way something could be reused when an allocation is made after the jemalloc key destroy function is called, but I might have missed something.
> >
> > This might be particular to the fact that my config uses pthread_key_create for the tsd data, but it might apply to other configs.
> >
> > Does this solution seem reasonable?
> 
> Unfortunately I didn't see this email until after the 4.0.0 release, because the mailing lists haven't been delivering email for the past two months (ouch).  Does this problem still exist with 4.0.0?
> 
> Thanks,
> Jason
>