400111 - nssArena code confuses leak testers, doesn't honor NSS_DISABLE_ARENA_FREE_LIST

Status: RESOLVED INVALID

(NSS :: Libraries, defect)

Description

[Nelson Bolyard (seldom reads bugmail)]

Background:

NSS's PORT_FreeArena destroys arena pools.  Normally it takes the memory
blocks (called arenas) from the arena pool and puts them onto a free list
of arenas, from which they are subsequently taken and reused.

But that's bad news for leak detection systems that try to report the 
stack that allocated a leaked block of memory.  The recycling of arenas
in the arena free list causes leaked arenas to appear to have been 
allocated by the first stack that originally allocated the arena from
the heap, no matter how many times it was "freed" to the arena free list
and then subsequently reused from there.  The stacks reported by the 
leak detection systems show the wrong stack for the allocator.

So PORT_FreeArena honors an environment variable named
NSS_DISABLE_ARENA_FREE_LIST.  When that variable is defined, regardless of
the string value, PORT_FreeArena frees arenas back to the heap by calling PL_FreeArenaPool, rather than putting the arenas onto the arena free list 
and recycling them by calling PL_FinishArenaPool.  

NSS also has a second implementation of arena pools, known as nssArenas.
nssArenas and PORT_Arenas share the same arena free list.  

The problem:

Unlike PORT_FreeArena, the function nssArena_Destroy pays no attention to 
NSS_DISABLE_ARENA_FREE_LIST.  nssArena_Destroy always calls PL_FinishArenaPool and never calls PL_FreeArenaPool.  So, even when NSS_DISABLE_ARENA_FREE_LIST 
is defined, nssArenas confuse memory leak checkers.  

The proposed fix: 
Change nssArena_Destroy (and nssArena_Create) to pay attention to 
NSS_DISABLE_ARENA_FREE_LIST and call PL_FinishArenaPool or PL_FreeArenaPool depending on its presence or absence, just as PORT_FreeArena does.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Sigh.  

I remembered it exactly backwards.
PL_FinishArenaPool  really frees the arenas in the pool
PL_FreeArenaPool    uses the arena free list (despite the name).

So, nssArena_Destroy (and nssArena_Create) NEVER use the free list.  

Comment 2

[Julien Pierre]

Nelson,

Is there any reason why we would never want the nssArena to use the free list ?