In Document::~Document, we're explicitly calling m_cachedResourceLoader.clear(). However, CSSFontSelector, which is owned by CSSStyleSelector, calls CachedResourceLoader::decrementRequestCount at the end of the destructor, accessing the freed-memory.
The following stack trace can be obtained on PerformanceTests/Parser/html5-full-renderer.html on debug builds of Mac port by setting a break point inside CachedResourceLoader::decrementRequestCount after hitting an assertion in CachedResourceLoader::~CachedResourceLoader. #0 0x1014e69aa in WebCore::CachedResourceLoader::decrementRequestCount at CachedResourceLoader.cpp:714 #1 0x1015d09b6 in WebCore::CSSFontSelector::clearDocument at CSSFontSelector.cpp:586 #2 0x1016bf3f0 in WebCore::CSSStyleSelector::~CSSStyleSelector at CSSStyleSelector.cpp:572 #3 0x1017a4cc7 in WTF::deleteOwnedPtr<WebCore::CSSStyleSelector> at OwnPtrCommon.h:54 #4 0x1017a4e26 in WTF::OwnPtr<WebCore::CSSStyleSelector>::~OwnPtr at OwnPtr.h:55 #5 0x10178292d in WebCore::Document::~Document at Document.cpp:580 #6 0x101bfc883 in WebCore::HTMLDocument::~HTMLDocument at HTMLDocument.cpp:91 #7 0x1017906c6 in WebCore::Document::guardDeref at Document.h:253 #8 0x10177ac28 in WebCore::Document::removedLastRef at Document.cpp:626 #9 0x1015b94dd in WebCore::TreeShared<WebCore::ContainerNode>::deref at TreeShared.h:79
Created attachment 125509 [details] Fixes the bug
It appears that making a reduction from html5-full-parser.html is pretty hard :( Maybe we can add it to LayoutTests as well as a crash-test?
http://code.google.com/p/chromium/issues/detail?id=112731
Comment on attachment 125509 [details] Fixes the bug Apparently our fuzz cluster had been finding crashes due to this bug. I'll add a test case using that.
Actually, this isn't a use-after-free since CSSFontSelector accesses CachedResourceLoader through Document.
Comment on attachment 125509 [details] Fixes the bug I'm sorry but turning this file into a DRT test appears to be really hard :( There appears to be some race condition in triggering this bug and it doesn't reproduce reliably in DRT. Also calling notifyDone or document.write or assigning some value to innerHTML/innerText also seem to make the test case not work :( I also tried embedding it inside an iframe and reload it several times but that doesn't seem to work either. <script> function check () { var mylink = document.getElementsByTagName('a'); location = mylink[0]; } </script><style>@font-face { font-family: "A"; src: url(); }* { font-family: A;</style> <body onload="check()"<linearGradient>A00A00A000AA0AA00AA0<a>
Comment on attachment 125509 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=125509&action=review > Source/WebCore/ChangeLog:11 > + No new tests but PerformanceTests/Parser/html5-full-render.html was crashing > + on performance bots due to this bug. Can we write an explicit LayoutTest for this crash?
Ah, I didn't see the earlier discussion. Nate might have some ideas about how to write a reliable test.
<rdar://problem/10815525>
japhet told me that he had tried to make a reduction but was unable to. At this point, I'd like to ask reviewers to have a look at my patch again. The fix is self-evidently correct as far as I'm concerned.
Comment on attachment 125509 [details] Fixes the bug Sigh. I'm worried that shuffling around these shutdown events will cause other crashes. Without adding tests, we don't know that we're continuously improving. We could just be cycling through problems.
(In reply to comment #12) > (From update of attachment 125509 [details]) > Sigh. I'm worried that shuffling around these shutdown events will cause other crashes. Without adding tests, we don't know that we're continuously improving. We could just be cycling through problems. I know. Hopefully the comment there makes it clear that we need to destroy clearStyleSelector first. In the long term, we really need to come up with a better mechanism to test the loader code.
Comment on attachment 125509 [details] Fixes the bug Clearing flags on attachment: 125509 Committed r107346: <http://trac.webkit.org/changeset/107346>
All reviewed patches have been landed. Closing bug.