Last Comment Bug 505108 - Early returns in Area::ParseCoords leak "char* cp"
: Early returns in Area::ParseCoords leak "char* cp"
Status: RESOLVED FIXED
: mlk, valgrind
Product: Core
Classification: Components
Component: Layout: Misc Code (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Randell Jesup [:jesup]
:
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks: 182554 mlk-fx7
  Show dependency treegraph
 
Reported: 2009-07-19 08:40 PDT by Jesse Ruderman
Modified: 2011-06-11 18:43 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Simple patch to correct this (1.44 KB, patch)
2011-06-11 00:38 PDT, Randell Jesup [:jesup]
roc: review+
rjesup: checkin+
Details | Diff | Splinter Review

Description Jesse Ruderman 2009-07-19 08:40:34 PDT
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)
Comment 2 Randell Jesup [:jesup] 2011-06-11 00:38:55 PDT
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().
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-06-11 02:42:15 PDT
Comment on attachment 538669 [details] [diff] [review]
Simple patch to correct this

Review of attachment 538669 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 4 Ed Morley [:emorley] 2011-06-11 14:35:25 PDT
http://hg.mozilla.org/mozilla-central/rev/11b4b4c73f36
Comment 5 Nicholas Nethercote [:njn] 2011-06-11 16:06:31 PDT
How significant was this leak?  Each leak is a single byte (plus heap block overhead, presumably), how often did they occur?
Comment 6 Randell Jesup [:jesup] 2011-06-11 18:19:38 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.