Early returns in Area::ParseCoords leak "char* cp"

RESOLVED FIXED in mozilla7

Status

()

Core
Layout: Misc Code
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: jesup)

Tracking

({mlk, valgrind})

Trunk
mozilla7
x86
Mac OS X
mlk, valgrind
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
Early returns in Area::ParseCoords leak "char* cp".  Regression from bug 182554, I believe.

2 bytes in 2 blocks are definitely lost in loss record 15 of 1,703
malloc (vg_replace_malloc.c:193)
NS_Alloc_P (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
ToNewCString(nsAString_internal const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
Area::ParseCoords(nsAString_internal const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
RectArea::ParseCoords(nsAString_internal const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageMap::AddArea(nsIContent*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageMap::SearchForAreas(nsIContent*, int&, int&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageMap::Init(nsIPresShell*, nsIFrame*, nsIDOMHTMLMapElement*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageFrame::GetImageMap(nsPresContext*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsImageFrame::PaintImage(nsIRenderingContext&, nsPoint, nsRect const&, imgIContainer*) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsDisplayImage::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
nsDisplayClip::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) (in /Users/jruderman/central/opt-obj/toolkit/library/XUL)
(Reporter)

Comment 1

8 years ago
http://hg.mozilla.org/mozilla-central/file/f92608198566/layout/generic/nsImageMap.cpp#l164
(Assignee)

Comment 2

6 years ago
Created attachment 538669 [details] [diff] [review]
Simple patch to correct this

Also corrects that it was freeing the ToNewCString() string with NS_Free() when it's supposed to use nsMemory::Free().
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
Attachment #538669 - Flags: review?(roc)
Comment on attachment 538669 [details] [diff] [review]
Simple patch to correct this

Review of attachment 538669 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #538669 - Flags: review?(roc) → review+

Updated

6 years ago
Blocks: 659860

Comment 4

6 years ago
http://hg.mozilla.org/mozilla-central/rev/11b4b4c73f36
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
How significant was this leak?  Each leak is a single byte (plus heap block overhead, presumably), how often did they occur?
(Assignee)

Comment 6

6 years ago
Normally single byte (rounded up to allocation chunk size), plus an all-whitespace string would be leaked.

No idea how frequent; it's obviously wrong and was hit by Jesse's tracemalloc run (it appears).  I'd *guess* infrequent to rare, but that's a guess.

Now fixed in any case.  Don't count on it for anything significant; if you have testcases you can look before and after, or add a debug when it's hit.
Target Milestone: mozilla7 → ---
(Assignee)

Updated

6 years ago
Attachment #538669 - Flags: checkin+

Updated

6 years ago
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.