890127 - Call u_init(UErrorCode*) and u_cleanup() to fix up the memory-reporting situation wrt ICU

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

You can call ICU functions without initializing the library, and you can exit without calling the cleanup function.  But doing so means you leak stuff, which tinderbox stats don't much like.  We should init and cleanup at some sort of reasonable points so as not to have ICU perturb stats (and to fix the leak-ish sort of behavior that's happening now, although realistically this stuff's going to exist for about the lifetime of the process).

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Initialize and cleanup ICU in JS_Init and JS_ShutDown

This depends on the patch in bug 896124, obviously.  I *think* the patch I reviewed from you that accounted for ICU allocations will play nicely with this -- u_cleanup() also clears the stored memory-management function pointers, so they have to be reset, but XPCJSRuntime should do that, so I think we're okay -- but you'll want to be aware of that issue when updating that patch.

Before:

[jwalden@find-waldo-now src]$ valgrind --leak-check=full dbg/js -e 'Intl'
==25032== Memcheck, a memory error detector
==25032== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==25032== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==25032== Command: dbg/js -e Intl
==25032== 
==25032== 
==25032== HEAP SUMMARY:
==25032==     in use at exit: 187,743 bytes in 181 blocks
==25032==   total heap usage: 6,785 allocs, 6,604 frees, 3,588,734 bytes allocated
==25032== 
==25032== 150,784 bytes in 1 blocks are possibly lost in loss record 170 of 170
==25032==    at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==25032==    by 0xBFC117: uprv_malloc_50 (cmemory.c:85)
==25032==    by 0xC4C4B0: icu_50::UMemory::operator new[](unsigned long) (uobject.cpp:70)
==25032==    by 0xCF6B3D: icu_50::isAvailableLocaleListInitialized(UErrorCode&) (coll.cpp:291)
==25032==    by 0xCF69F0: icu_50::Collator::getAvailableLocales(int&) (coll.cpp:509)
==25032==    by 0xBE78F7: ucol_countAvailable_50 (ucol_res.cpp:748)
==25032==    by 0x4F12FC: intl_availableLocales(JSContext*, int (*)(), char const* (*)(int), JS::MutableHandle<JS::Value>) (Intl.cpp:435)
==25032==    by 0x4F11D3: js::intl_Collator_availableLocales(JSContext*, unsigned int, JS::Value*) (Intl.cpp:738)
==25032==    by 0x4D2BE4: js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:226)
==25032==    by 0x4D24B9: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:477)
==25032==    by 0x4CCC43: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2500)
==25032==    by 0x4C69E3: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:434)
==25032== 
==25032== LEAK SUMMARY:
==25032==    definitely lost: 0 bytes in 0 blocks
==25032==    indirectly lost: 0 bytes in 0 blocks
==25032==      possibly lost: 150,784 bytes in 1 blocks
==25032==    still reachable: 36,959 bytes in 180 blocks
==25032==         suppressed: 0 bytes in 0 blocks
==25032== Reachable blocks (those to which a pointer was found) are not shown.
==25032== To see them, rerun with: --leak-check=full --show-reachable=yes
==25032== 
==25032== For counts of detected and suppressed errors, rerun with: -v
==25032== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2)

After:

[jwalden@find-waldo-now src]$ valgrind --leak-check=full dbg/js -e 'Intl'
==25246== Memcheck, a memory error detector
==25246== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==25246== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==25246== Command: dbg/js -e Intl
==25246== 
==25246== 
==25246== HEAP SUMMARY:
==25246==     in use at exit: 0 bytes in 0 blocks
==25246==   total heap usage: 6,787 allocs, 6,787 frees, 3,589,358 bytes allocated
==25246== 
==25246== All heap blocks were freed -- no leaks are possible
==25246== 
==25246== For counts of detected and suppressed errors, rerun with: -v
==25246== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)

Comment 2

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c77439f57dfc

Comment 3

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c77439f57dfc