499607 - Leak nsLineList with -moz-column

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: 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)

Comment 1

[Jesse Ruderman]

The patch in bug 499611 lets trace-refcnt detect this leak, in case you don't want to use valgrind ;)

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: patch

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.

Comment 3

[fantasai]

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.

Comment 4

[David Baron :dbaron:]

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.

Comment 5

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/mozilla-central/rev/21d443acefc5

Comment 6

[Timothy Nikkel (:tnikkel)]

(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.