Bug 150728 – leaks seen in fast/css/variables tests

• Component: CSS
• Status: RESOLVED FIXED

[Joseph Pecoraro]

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

[Darin Adler]

I suspect https://trac.webkit.org/changeset/201608 fixed this. However, I will wait and test.

[Darin Adler]

Nope, still see leaks.

[Darin Adler]

Created attachment 280523 [details]
Patch

[Darin Adler]

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.

[Anders Carlsson]

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.

[Darin Adler]

Committed r201690: <http://trac.webkit.org/changeset/201690>