Closed Bug 499607 Opened 11 years ago Closed 11 years ago

Leak nsLineList with -moz-column

Categories

(Core :: Layout, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: jruderman, Assigned: tnikkel)

References

Details

(Keywords: memory-leak, testcase, valgrind)

Attachments

(2 files)

Attached file testcase
This testcase might just be a reduction of layout/base/crashtests/423107-1.xhtml.

valgrind 
    --leak-check=full
    --auto-run-dsymutil=yes
    --track-origins=yes 
    ~/central/debug-obj/dist/MinefieldDebug.app/Contents/MacOS/firefox-bin 
    -P vgrind b1.html 

8 bytes in 1 blocks are definitely lost in loss record 118 of 1,965
   at 0x12B7F: operator new(unsigned long) (vg_replace_malloc.c:218)
   by 0x21B97429: nsBlockFrame::PushLines(nsBlockReflowState&, nsLineList_iterator) (nsBlockFrame.cpp:4292)
   by 0x21B97D9C: nsBlockFrame::PlaceLine(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFloatManager::SavedState*, nsRect&, int&, int*) (nsBlockFrame.cpp:4192)
   by 0x21B9A703: nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, int*, LineReflowStatus*, int) (nsBlockFrame.cpp:3663)
   by 0x21B9A923: nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, int*) (nsBlockFrame.cpp:3378)
   by 0x21B9AEE9: nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) (nsBlockFrame.cpp:2422)
   by 0x21B9B73F: nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) (nsBlockFrame.cpp:1919)
   by 0x21B9EB50: nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) (nsBlockFrame.cpp:958)
   by 0x21BADE3C: nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) (nsContainerFrame.cpp:825)
   by 0x21BAAFF9: nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, int, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) (nsColumnSetFrame.cpp:669)
   by 0x21BABEBD: nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) (nsColumnSetFrame.cpp:1030)
   by 0x21BA2914: nsBlockReflowContext::ReflowBlock(nsRect const&, int, nsCollapsingMargin&, int, int, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) (nsBlockReflowContext.cpp:310)
Blocks: 499611
The patch in bug 499611 lets trace-refcnt detect this leak, in case you don't want to use valgrind ;)
Attached patch patchSplinter Review
This is based on top of the patch in bug 514634, not because they are related, but because they coincidentally change code right next to each other.
Assignee: nobody → tnikkel
Attachment #398911 - Flags: review?(fantasai.bugs)
Comment on attachment 398911 [details] [diff] [review]
patch

I'd like dbaron to do an sr here, to make sure this is the right approach to the problem and some kind of api refactoring isn't wanted. Looks good otherwise.
Attachment #398911 - Flags: superreview?(dbaron)
Attachment #398911 - Flags: review?(fantasai.bugs)
Attachment #398911 - Flags: review+
Comment on attachment 398911 [details] [diff] [review]
patch

sr=dbaron

In theory (and to match the C++ standard library's list<T>, after which it is modeled) nsLineList::erase should call the line box's destructor (as should pop_front, pop_back, and clear).  However, nsLineBox doesn't have a useful destructor; it has a destroy method that takes a pres shell argument, which makes that a good bit harder.  So this is ok, at least for now.
Attachment #398911 - Flags: superreview?(dbaron) → superreview+
http://hg.mozilla.org/mozilla-central/rev/21d443acefc5
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
(In reply to comment #4)
> In theory (and to match the C++ standard library's list<T>, after which it is
> modeled) nsLineList::erase should call the line box's destructor ...

I'm confused, this bug is about leaking nsLineList itself, not any of the nodes it contains. So I don't understand how your comment applies here.
You need to log in before you can comment on or make changes to this bug.