599773 - Memory leak in JSCompartment::init()

Status: RESOLVED DUPLICATE of bug 599801

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

The memory leak shows up in all the trace-tests that run under Valgrind, it looks like the leak always occurs:

==16742== 384 bytes in 1 blocks are definitely lost in loss record 1 of 2
==16742==    at 0x4CA1C1C: malloc (vg_replace_malloc.c:195)
==16742==    by 0x805A3DC: js_malloc (jsutil.h:193)
==16742==    by 0x80746B0: js::SystemAllocPolicy::malloc(unsigned int) (jstl.h:253)
==16742==    by 0x80E8875: js::detail::HashTable<js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::Entry, js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::createTable(js::SystemAllocPolicy&, unsigned int) (jshashtable.h:295)
==16742==    by 0x80E85E3: js::detail::HashTable<js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::Entry, js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::init(unsigned int) (jshashtable.h:348)
==16742==    by 0x80E81B7: js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::init(unsigned int) (jshashtable.h:807)
==16742==    by 0x80E7551: JSCompartment::init() (jscompartment.cpp:82)
==16742==    by 0x805DA49: JSRuntime::init(unsigned int) (jsapi.cpp:620)
==16742==    by 0x8069766: JS_Init (jsapi.cpp:756)
==16742==    by 0x805619A: main (js.cpp:5336)
==16742== 


Looks like the patch for bug 558861 is to blame.

Comment 1

[Andreas Gal :gal]

Gregor, is this the default compartment that isn't de-allocated?

Comment 2

[Gregor Wagner [:gwagner]]

It looks more the WrapperMap crossCompartmentWrappers is not de-allocated.

Comment 3

[Gregor Wagner [:gwagner]]

It looks like this happens for all Compartments even if the deconstructors are called. A simple shell start shows the leak for the defaultCompartment and the compartment generated in for the shell:

==31198== 384 bytes in 1 blocks are definitely lost in loss record 701 of 799
==31198==    at 0x10065ED65: malloc (vg_replace_malloc.c:236)
==31198==    by 0x1001611C9: js_malloc (in ./js)
==31198==    by 0x100170AE0: js::SystemAllocPolicy::malloc(unsigned long) (in ./js)
==31198==    by 0x10009F5AB: js::detail::HashTable<js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::Entry, js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::createTable(js::SystemAllocPolicy&, unsigned int) (in ./js)
==31198==    by 0x1000A0022: js::detail::HashTable<js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::Entry, js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::init(unsigned int) (in ./js)
==31198==    by 0x1000A008E: js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::init(unsigned int) (in ./js)
==31198==    by 0x10009E6F1: JSCompartment::init() (in ./js)
==31198==    by 0x10008C513: js::gc::NewCompartment(JSContext*, JSPrincipals*) (in ./js)
==31198==    by 0x10001D282: JS_NewCompartmentAndGlobalObject (in ./js)
==31198==    by 0x1000054C4: NewGlobalObject(JSContext*, JSAutoCrossCompartmentCall&) (in ./js)
==31198==    by 0x10000D581: shell(JSContext*, int, char**, char**) (in ./js)
==31198==    by 0x10000D707: main (in ./js)
==31198== 
==31198== 384 bytes in 1 blocks are definitely lost in loss record 702 of 799
==31198==    at 0x10065ED65: malloc (vg_replace_malloc.c:236)
==31198==    by 0x1001611C9: js_malloc (in ./js)
==31198==    by 0x100170AE0: js::SystemAllocPolicy::malloc(unsigned long) (in ./js)
==31198==    by 0x10009F5AB: js::detail::HashTable<js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::Entry, js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::createTable(js::SystemAllocPolicy&, unsigned int) (in ./js)
==31198==    by 0x1000A0022: js::detail::HashTable<js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::Entry, js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::init(unsigned int) (in ./js)
==31198==    by 0x1000A008E: js::HashMap<js::Value, js::Value, js::WrapperHasher, js::SystemAllocPolicy>::init(unsigned int) (in ./js)
==31198==    by 0x10009E6F1: JSCompartment::init() (in ./js)
==31198==    by 0x10001FC4B: JSRuntime::init(unsigned int) (in ./js)
==31198==    by 0x10002A5DB: JS_Init (in ./js)
==31198==    by 0x10000D693: main (in ./js)

Comment 4

[Nicholas Nethercote [inactive]]

Luke, I suspect this may be a leak somehow in or relating to AllocPolicy, but that code is complex and I couldn't work it out.  Could you take a look?  Thanks.

Comment 5

[Luke Wagner [:luke]]

Ah, it seems like the optimizer has determined that the free() is always called directly before process exit and has optimized it away.

j/k, init() is being called twice on the same compartment (rt->defaultCompartment): once from JSRuntime() and once from js_InitGC ;-)

Perhaps whoever patches this can add an assertion that init isn't called twice on a hashtable (stick in a JS_ASSERT(table == NULL) in HashTable::init).

Comment 6

[Gregor Wagner [:gwagner]]

(In reply to comment #5)
> Ah, it seems like the optimizer has determined that the free() is always called
> directly before process exit and has optimized it away.
> 
> j/k, init() is being called twice on the same compartment
> (rt->defaultCompartment): once from JSRuntime() and once from js_InitGC ;-)
> 
> Perhaps whoever patches this can add an assertion that init isn't called twice
> on a hashtable (stick in a JS_ASSERT(table == NULL) in HashTable::init).

Oh the same with all other compartments. We call init twice... I will fix it with the patch in bug 599801.