Bug 150728 - leaks seen in fast/css/variables tests
Summary: leaks seen in fast/css/variables tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-30 12:41 PDT by Joseph Pecoraro
Modified: 2016-06-04 16:01 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.90 KB, patch)
2016-06-04 13:31 PDT, Darin Adler
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-10-30 12:41:31 PDT
* SUMMARY
CSSParserValueList leaks seen in fast/css/variables tests.

* STEPS TO REPRODUCE
shell> run-webkit-tests --leaks -1 fast/css/variables

* NOTES
- Debug build will probably get you better allocation stacks then what I got.

* LEAK
Leak: 0x7f85e0c30b60  size=160  zone: DefaultMallocZone_0x10bf86000
	0x00000000 0x00000000 0xe0c30b78 0x00007f85 	........x.......
	0x00000004 0x00000001 0x00000000 0x3ff00000 	...............?
	0x00000000 0x3ff00000 0x74b29a50 0x00007fff 	.......?P..t....
	0x00000005 0x00007fff 0x00000010 0x005a000f 	..............Z.
	0x00000000 0x00000000 0x5e0c2ffb 0x000507f8 	........./.^....
	0x00000002 0x00000011 0xe0c30bc4 0x00007f85 	................
	0x0164d908 0x706d7564 0x656c6553 0x6f697463 	..d.dumpSelectio
	0x6365526e 0x00000074 0xe0c30be4 0x00007f85 	nRect...........
	...
	Call stack: [thread 0x7fff733f9000]: 
        | 0x2 
        | start 
        | DumpRenderTreeMain(int, char const**) DumpRenderTree.mm:1430 
        | dumpRenderTree(int, char const**) DumpRenderTree.mm:1294 
        | runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) DumpRenderTree.mm:2037 
        | CFRunLoopRunSpecific 
        | __CFRunLoopRun 
        | __CFRunLoopDoSources0 
        | __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ 
        | MultiplexerSource::_perform(void*) 
        | MultiplexerSource::perform() 
        | RunloopBlockContext::perform() 
        | CFArrayApplyFunction 
        | RunloopBlockContext::_invoke_block(void const*, void*) 
        | _dispatch_block_invoke 
        | _dispatch_client_callout 
        | ___ZN27URLConnectionClient_Classic18_withDelegateAsyncEPKcU13block_pointerFvP16_CFURLConnectionPK33CFURLConnectionClientCurrent_VMaxE_block_invoke_2 
        | ___ZN27URLConnectionClient_Classic26_delegate_didFinishLoadingEU13block_pointerFvvE_block_invoke 
        | -[NSURLConnectionInternal _withActiveConnectionAndDelegate:] 
        | -[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:] 
        | __65-[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:]_block_invoke 
        | WebCore::SubresourceLoader::didFinishLoading(double) ResourceLoader.h:154 
        | WebCore::CachedRawResource::finishLoading(WebCore::SharedBuffer*) CachedRawResource.cpp:104 
        | WebCore::CachedResource::checkNotify() CachedResourceClientWalker.h:51 
        | WebCore::DocumentLoader::finishedLoading(double) ResourceErrorBase.h:42 
        | WebCore::DocumentWriter::end() RefPtr.h:71 
        | WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter&) StdLibExtras.h:358 
        | WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl>&&) DocumentParser.h:71 
        | WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) HTMLTokenizer.h:245 
        | WebCore::HTMLDocumentParser::constructTreeFromHTMLToken(WebCore::HTMLTokenizer::TokenPtr&) HTMLDocumentParser.cpp:321 
        | WebCore::HTMLTreeBuilder::constructTree(WebCore::AtomicHTMLToken&) HTMLTreeBuilder.cpp:343 
        | WebCore::HTMLTreeBuilder::processEndTag(WebCore::AtomicHTMLToken&) HTMLTreeBuilder.cpp:2144 
        | WebCore::HTMLElementStack::pop() memory:2636 
        | WebCore::HTMLStyleElement::finishParsingChildren() HTMLStyleElement.cpp:90 
        | WebCore::InlineStyleSheetOwner::finishParsingChildren(WebCore::Element&) StdLibExtras.h:358 
        | WebCore::InlineStyleSheetOwner::createSheet(WebCore::Element&, WTF::String const&) InlineStyleSheetOwner.cpp:157 
        | WebCore::StyleSheetContents::parseStringAtPosition(WTF::String const&, WTF::TextPosition const&, bool) StyleSheetContents.cpp:338 
        | WebCore::CSSParser::parseSheet(WebCore::StyleSheetContents*, WTF::String const&, WTF::TextPosition const&, WTF::Vector<WTF::RefPtr<WebCore::CSSRuleSourceData>, 0ul, WTF::CrashOnOverflow, 16ul>*, bool) CSSParser.cpp:462 
        | cssyyparse(WebCore::CSSParser*) CSSParserValues.h:141 
        | malloc 
        | malloc_zone_malloc 

* NOTES
You may consider looking at:

    CSSValueList::buildParserValueSubstitutingVariables
    CSSValueList::buildParserValueListSubstitutingVariables

I think in error cases a CSSParserValueList may not be getting deleted properly. But there may be other issues.
Comment 1 Darin Adler 2016-06-03 19:30:49 PDT
I suspect https://trac.webkit.org/changeset/201608 fixed this. However, I will wait and test.
Comment 2 Darin Adler 2016-06-03 20:07:04 PDT
Nope, still see leaks.
Comment 3 Darin Adler 2016-06-04 13:31:06 PDT
Created attachment 280523 [details]
Patch
Comment 4 Darin Adler 2016-06-04 13:32:19 PDT
I suspect the CSSParserValueList leaks were fixed by https://trac.webkit.org/changeset/201608 -- the patch fixes the String leaks that were still happening. Together the two changes have fixed all the leaks.
Comment 5 Anders Carlsson 2016-06-04 15:52:43 PDT
Comment on attachment 280523 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=280523&action=review

> Source/WebCore/css/CSSPrimitiveValue.cpp:285
> +        return true;
> +    default:
> +        return false;

I'd list all the other types here instead of having a default case so we'll fail to compile if someone adds another type.
Comment 6 Darin Adler 2016-06-04 16:01:26 PDT
Committed r201690: <http://trac.webkit.org/changeset/201690>