Closed Bug 487967 Opened 11 years ago Closed 11 years ago

occasional leak of array or hash table storage allocation in js_AddLocal


(Core :: JavaScript Engine, defect)

Not set





(Reporter: dbaron, Assigned: dbaron)



(Keywords: fixed1.9.1, memory-leak, Whiteboard: fixed-in-tracemonkey)


(1 file)

Attached patch patchSplinter Review
We occasionally leak either the array or the hash table storage allocated in js_AddLocal (in theory, potentially the hash table itself, but I didn't see that) because DestroyLocalNames has an out-of-sync copy of the JS_GET_LOCAL_NAME_COUNT macro that became out-of-sync when (bug 445262) changed the macro to include upvars as well.

This patch corrects the calculation of n (which incorrectly did not include fun->u.i.upvars) so that we free the right type of object (nothing, array, or hashtable).  (This didn't actually cause any problems other than a leak because the free-the-array code didn't crash on the hashtable; instead it freed the table without freeing the entry store.)

(If I land this, does it need to be landed in tracemonkey first?
Attachment #372249 - Flags: review?(brendan)
(Note that I saw this leak just starting up Firefox; most of the leaks had fun_xdrObject as the caller of js_AddLocal.)
Duplicate of this bug: 487191
Comment on attachment 372249 [details] [diff] [review]

This looks like the only such bug, from `grep -n nargs *.cpp | grep nvars`. Thanks for the fix. I'll do the tm landing -- let me know if you want me to do m-c too.

Attachment #372249 - Flags: review?(brendan) → review+
Flags: blocking1.9.1?
Fixed in tm:

Whiteboard: fixed-in-tracemonkey
No longer blocks: upvar1
Blocks: upvar1, upvar2
I pushed it to mozilla-central as well:
Closed: 11 years ago
Resolution: --- → FIXED
This should block for sure.

Flags: blocking1.9.1? → blocking1.9.1+
This wouldn't have caused a Tp3 regression unless there's something seriously wrong with our memory allocator.
You need to log in before you can comment on or make changes to this bug.