Closed Bug 487967 Opened 11 years ago Closed 11 years ago

occasional leak of array or hash table storage allocation in js_AddLocal

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.1b4

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

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

Attachments

(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 http://hg.mozilla.org/mozilla-central/rev/5bc82976d48b (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]
patch

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.

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

http://hg.mozilla.org/tracemonkey/rev/2a8287df3669

/be
Whiteboard: fixed-in-tracemonkey
No longer blocks: upvar1
Blocks: upvar1, upvar2
I pushed it to mozilla-central as well:

http://hg.mozilla.org/mozilla-central/rev/2a8287df3669
http://hg.mozilla.org/mozilla-central/rev/9bda3eab3b2d
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
This should block for sure.

/be
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.