Nicholas Nethercote [inactive] Assignee
	Description • 14 years ago

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.

	Andreas Gal :gal
	Comment 1 • 14 years ago

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

	Gregor Wagner [:gwagner]
	Comment 2 • 14 years ago

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

	Gregor Wagner [:gwagner]
	Comment 3 • 14 years ago

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)

	Nicholas Nethercote [inactive] Assignee
	Comment 4 • 14 years ago

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.

	Luke Wagner [:luke]
	Comment 5 • 14 years ago

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).

	Gregor Wagner [:gwagner]
	Comment 6 • 14 years ago

(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.