Bug 41281 - HTML5 Regression: Crash in insert()
Summary: HTML5 Regression: Crash in insert()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://www.freshersworld.com/
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-28 09:16 PDT by Tony Gentilcore
Modified: 2010-06-28 14:24 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.86 KB, patch)
2010-06-28 10:21 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Gentilcore 2010-06-28 09:16:39 PDT
http://build.chromium.org/buildbot/waterfall.fyi/builders/WebKit%20Reliability/builds/387/steps/reliability:%20partial%20result%20of%20current%20build/logs/stdio

REGRESSION: NEW crash stack traces found
--------------------
Repro information:
Unfiltered URL: http://www.freshersworld.com/

Stack trace:
chrome_2580000!WebCore::HTMLDocumentParser::insert+0x78 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 228]
chrome_2580000!WebCore::Document::write+0x46 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\dom\document.cpp @ 2032]
chrome_2580000!WebCore::V8HTMLDocument::writeCallback+0x3d [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\custom\v8htmldocumentcustom.cpp @ 115]
chrome_2580000!v8::internal::HandleApiCallHelper<0>+0x197 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 971]
chrome_2580000!v8::internal::Builtin_HandleApiCall+0xf [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 988]
chrome_2580000!v8::internal::Invoke+0xc2 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 96]
chrome_2580000!v8::internal::Execution::Call+0x26 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 121]
chrome_2580000!v8::Script::Run+0x157 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\api.cc @ 1247]
chrome_2580000!WebCore::V8Proxy::runScript+0xfd [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 452]
chrome_2580000!WebCore::V8Proxy::evaluate+0x16b [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 404]
chrome_2580000!WebCore::ScriptController::evaluate+0x111 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\scriptcontroller.cpp @ 242]
chrome_2580000!WebCore::ScriptController::executeScript+0x8c [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\scriptcontrollerbase.cpp @ 62]
chrome_2580000!WebCore::HTMLScriptRunner::executeScript+0x48 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 160]
chrome_2580000!WebCore::HTMLScriptRunner::runScript+0x1a4 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 277]
chrome_2580000!WebCore::HTMLScriptRunner::execute+0x13 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 187]
chrome_2580000!WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder+0x92 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 153]
chrome_2580000!WebCore::HTMLDocumentParser::pumpTokenizer+0xb0 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 180]
chrome_2580000!WebCore::HTMLDocumentParser::insert+0x68 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 227]
chrome_2580000!WebCore::Document::write+0x46 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\dom\document.cpp @ 2032]
chrome_2580000!WebCore::V8HTMLDocument::writeCallback+0x3d [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\custom\v8htmldocumentcustom.cpp @ 115]
chrome_2580000!v8::internal::HandleApiCallHelper<0>+0x197 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 971]
chrome_2580000!v8::internal::Builtin_HandleApiCall+0xf [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\builtins.cc @ 988]
WARNING: Frame IP not in any known module. Following frames may be wrong.
0x61d010e
0x66ba8d2
chrome_2580000!v8::internal::Invoke+0xc2 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 96]
chrome_2580000!v8::internal::Execution::Call+0x26 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\execution.cc @ 121]
chrome_2580000!v8::Script::Run+0x157 [c:\b\slave\webkit-rel-reliability-builder\build\src\v8\src\api.cc @ 1247]
chrome_2580000!WebCore::V8Proxy::runScript+0xfd [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 452]
chrome_2580000!WebCore::V8Proxy::evaluate+0x16b [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\v8proxy.cpp @ 404]
chrome_2580000!WebCore::ScriptController::evaluate+0x111 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\v8\scriptcontroller.cpp @ 242]
chrome_2580000!WebCore::ScriptController::executeScript+0x8c [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\bindings\scriptcontrollerbase.cpp @ 62]
chrome_2580000!WebCore::HTMLScriptRunner::executeScript+0x48 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 160]
chrome_2580000!WebCore::HTMLScriptRunner::executePendingScript+0xfd [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 141]
chrome_2580000!WebCore::HTMLScriptRunner::executeParsingBlockingScripts+0x50 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmlscriptrunner.cpp @ 207]
chrome_2580000!WebCore::HTMLDocumentParser::executeScriptsWaitingForStylesheets+0x24 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmldocumentparser.cpp @ 395]
chrome_2580000!WebCore::Document::removePendingSheet+0x3a [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\dom\document.cpp @ 2623]
chrome_2580000!WebCore::HTMLLinkElement::sheetLoaded+0x3d [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmllinkelement.cpp @ 355]
chrome_2580000!WebCore::CSSStyleSheet::checkLoaded+0x31 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\css\cssstylesheet.cpp @ 214]
chrome_2580000!WebCore::HTMLLinkElement::setCSSStyleSheet+0x329 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\html\htmllinkelement.cpp @ 340]
chrome_2580000!WebCore::CachedCSSStyleSheet::checkNotify+0x70 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\cachedcssstylesheet.cpp @ 116]
chrome_2580000!WebCore::CachedCSSStyleSheet::data+0x13a [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\cachedcssstylesheet.cpp @ 106]
chrome_2580000!WebCore::Loader::Host::didFinishLoading+0xd4 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\loader.cpp @ 407]
chrome_2580000!WebCore::SubresourceLoader::didFinishLoading+0x27 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\subresourceloader.cpp @ 196]
chrome_2580000!WebCore::ResourceLoader::didFinishLoading+0x7 [c:\b\slave\webkit-rel-reliability-builder\build\src\third_party\webkit\webcore\loader\resourceloader.cpp @ 444]
chrome_2580000!webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest+0x18c [c:\b\slave\webkit-rel-reliability-builder\build\src\webkit\glue\weburlloader_impl.cc @ 583]
chrome_2580000!ResourceDispatcher::OnRequestComplete+0x8e [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\resource_dispatcher.cc @ 469]
chrome_2580000!IPC::MessageWithTuple<Tuple3<int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >::Dispatch<ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>+0x5b [c:\b\slave\webkit-rel-reliability-builder\build\src\ipc\ipc_message_utils.h @ 1044]
chrome_2580000!ResourceDispatcher::DispatchMessageW+0xa1 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\resource_dispatcher.cc @ 536]
chrome_2580000!ResourceDispatcher::OnMessageReceived+0x287 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\resource_dispatcher.cc @ 302]
chrome_2580000!ChildThread::OnMessageReceived+0x1a [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\common\child_thread.cc @ 124]
chrome_2580000!RunnableMethod<`anonymous namespace'::JobTracker,void (__thiscall A0x926de479::JobTracker::*)(std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const &),Tuple1<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > > >::Run+0x17 [c:\b\slave\webkit-rel-reliability-builder\build\src\base\task.h @ 323]
chrome_2580000!MessageLoop::RunTask+0x10d [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 341]
chrome_2580000!MessageLoop::DoWork+0x12f [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 460]
chrome_2580000!base::MessagePumpDefault::Run+0x117 [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_pump_default.cc @ 50]
chrome_2580000!MessageLoop::RunInternal+0x92 [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 214]
chrome_2580000!MessageLoop::Run+0x5a [c:\b\slave\webkit-rel-reliability-builder\build\src\base\message_loop.cc @ 165]
chrome_2580000!RendererMain+0x33f [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\renderer\renderer_main.cc @ 294]
chrome_2580000!ChromeMain+0xab9 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\app\chrome_dll_main.cc @ 760]
chrome!MainDllLoader::Launch+0x191 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\app\client_util.cc @ 257]
chrome!wWinMain+0x97 [c:\b\slave\webkit-rel-reliability-builder\build\src\chrome\app\chrome_exe_main.cc @ 47]
chrome!__tmainCRTStartup+0x176 [f:\dd\vctools\crt_bld\self_x86\crt\src\crt0.c @ 324]
kernel32!RegisterWaitForInputIdle+0x49
Comment 1 Adam Barth 2010-06-28 09:25:50 PDT
Tony, are you investigating this crash?

@eseidel: How do we get into a situation in which executeScriptsWaitingForStylesheets is called?  After that, it looks like two nested calls to document.write.
Comment 2 Tony Gentilcore 2010-06-28 09:28:29 PDT
(In reply to comment #1)
> Tony, are you investigating this crash?

Sorry, I can't look into it until after lunch today. If it is still there, I'll tackle it then.

> 
> @eseidel: How do we get into a situation in which executeScriptsWaitingForStylesheets is called?  After that, it looks like two nested calls to document.write.
Comment 3 Adam Barth 2010-06-28 09:29:58 PDT
Ok.  I'll take a look.
Comment 4 Adam Barth 2010-06-28 10:06:59 PDT
Well, I can create this call stack using the following test case:

== repro.html ==

<link rel="stylesheet" href="http://webblaze.org/">
<body>
1
<script src="ext.js"></script>
5
Done!

== ext.js ==

document.write(2);
document.write(" <script>document.write(3)</script> ");
document.write(4);

However, nothing bad happens.  My theory is as follows: instead of 3, we do something that tricks endIfDelayed into actually calling end(), which deletes the HTMLDocumentParser, which causes the decrement of the script nesting level to be twiddling unallocated memory.  I'm pretty sure there's a bug on that line of code, so I'm going to post a patch to fix it.
Comment 5 Adam Barth 2010-06-28 10:21:39 PDT
Created attachment 59907 [details]
Patch
Comment 6 Alexey Proskuryakov 2010-06-28 10:32:42 PDT
This is just a crasher bug, not a site being incompatible with HTML5, so this shouldn't block bug 41115.
Comment 7 Eric Seidel (no email) 2010-06-28 13:13:47 PDT
Comment on attachment 59907 [details]
Patch

I definitely like this change.

WebCore/html/HTMLDocumentParser.h:112
 +      bool shouldDelayEnd() const { return inWrite() || isWaitingForScripts() || inScriptExecution() || isScheduledForResume(); }
Seems sightly silly to put this in the header.  Generally the .cpp is better/cleaner.

I was sad when I broke write into insert and append because the code sharing previously seemed good.  It's still good.  But I wonder how it could be better.
Comment 8 WebKit Commit Bot 2010-06-28 13:30:49 PDT
Comment on attachment 59907 [details]
Patch

Clearing flags on attachment: 59907

Committed r62033: <http://trac.webkit.org/changeset/62033>
Comment 9 WebKit Commit Bot 2010-06-28 13:30:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Adam Barth 2010-06-28 14:16:22 PDT
> WebCore/html/HTMLDocumentParser.h:112
>  +      bool shouldDelayEnd() const { return inWrite() || isWaitingForScripts() || inScriptExecution() || isScheduledForResume(); }
> Seems sightly silly to put this in the header.  Generally the .cpp is better/cleaner.

Yeah, I don't know if we have a consistent way of making those decisions.
Comment 11 WebKit Review Bot 2010-06-28 14:24:06 PDT
http://trac.webkit.org/changeset/62033 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/62032
http://trac.webkit.org/changeset/62033
http://trac.webkit.org/changeset/62034
http://trac.webkit.org/changeset/62031