52945 2011-01-21 19:50:12 -0800 crash @ WebCore::ResourceLoader::didCancel(WebCore::ResourceError const &) 2011-01-22 11:23:34 -0800 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) PC All RESOLVED DUPLICATE 51357 P1 Normal --- 0 rtenneti webkit-unassigned abarth ap beidson jar webkit.review.bot oldest_to_newest 338478 0 rtenneti 2011-01-21 19:50:12 -0800 Logged into my personal yahoo email a/c and opened an email and print it. Displays printable version of email in a new window and select printer dialog gets displayed. Click on "Cancel" of that dialog, sometimes it crashes (duplicated it in Chrome build Beta and Dev). Stack trace: ########### Thread 0 *CRASHED* ( EXCEPTION_ACCESS_VIOLATION_READ @ 0x000006c4 ) 0x0225162f [chrome.dll - resourceloader.cpp:341] WebCore::ResourceLoader::didCancel(WebCore::ResourceError const &) 0x02250e03 [chrome.dll - subresourceloader.cpp:231] WebCore::SubresourceLoader::didCancel(WebCore::ResourceError const &) 0x02251716 [chrome.dll - resourceloader.cpp:364] WebCore::ResourceLoader::cancel(WebCore::ResourceError const &) 0x022516c1 [chrome.dll - resourceloader.cpp:354] WebCore::ResourceLoader::cancel() 0x02179605 [chrome.dll - documentloader.cpp:64] WebCore::cancelAll 0x02179cc6 [chrome.dll - documentloader.cpp:251] WebCore::DocumentLoader::stopLoading(WebCore::DatabasePolicy) 0x0210316e [chrome.dll - frameloader.cpp:1707] WebCore::FrameLoader::stopAllLoaders(WebCore::DatabasePolicy) 0x023fdb04 [chrome.dll - webframeimpl.cpp:962] WebKit::WebFrameImpl::stopLoading() 0x024112e3 [chrome.dll - chromeclientimpl.cpp:427] WebKit::ChromeClientImpl::closeWindowSoon() 0x0233860a [chrome.dll - v8domwindow.cpp:2636] WebCore::DOMWindowInternal::closeCallback 0x028e4d86 [chrome.dll - builtins.cc:983] v8::internal::HandleApiCallHelper<0> 0x028e507f [chrome.dll + 0x00cb507f] 0x05e4d458 Thread 1 0x7c90e514 [ntdll.dll + 0x0000e514] KiFastSystemCallRet 0x7c90df49 [ntdll.dll + 0x0000df49] NtWaitForMultipleObjects 0x7c80958f [kernel32.dll + 0x0000958f] CreateFileMappingA 0x77df8630 [advapi32.dll + 0x00028630] WmipEventPump 0x7c80b728 [kernel32.dll + 0x0000b728] BaseThreadStart Thread 2 0x7c90e514 [ntdll.dll + 0x0000e514] KiFastSystemCallRet 0x7c90da49 [ntdll.dll + 0x0000da49] ZwRemoveIoCompletion 0x7c80a7e5 [kernel32.dll + 0x0000a7e5] GetQueuedCompletionStatus 0x01d0b56b [chrome.dll - message_pump_win.cc:518] base::MessagePumpForIO::GetIOItem(unsigned long,base::MessagePumpForIO::IOItem *) 0x01d0b4b7 [chrome.dll - message_pump_win.cc:487] base::MessagePumpForIO::WaitForIOCompletion(unsigned long,base::MessagePumpForIO::IOHandler *) 0x01d0b45f [chrome.dll - message_pump_win.cc:465] base::MessagePumpForIO::DoRunLoop() 0x01d0aefe [chrome.dll - message_pump_win.cc:51] base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate *,base::MessagePumpWin::Dispatcher *) 0x01d0ad43 [chrome.dll - message_pump_win.h:80] base::MessagePumpWin::Run(base::MessagePump::Delegate *) 0x01cf5d13 [chrome.dll - message_loop.cc:258] MessageLoop::RunInternal() 0x01cf5c91 [chrome.dll - message_loop.cc:230] MessageLoop::RunHandler() 0x01cf5c3f [chrome.dll - message_loop.cc:208] MessageLoop::Run() 0x0270d40d [chrome.dll - thread.cc:140] base::Thread::Run(MessageLoop *) 0x0270d4b9 [chrome.dll - thread.cc:164] base::Thread::ThreadMain() 0x01cfd757 [chrome.dll - platform_thread_win.cc:26] `anonymous namespace'::ThreadFunc(void *) 0x7c80b728 [kernel32.dll + 0x0000b728] BaseThreadStart ------- From japhet@chromium.org noticed the following: We're crashing in ResourceLoader::didCancel() because we're assuming that ResourceLoader::m_documentLoader is valid. The stack below is from within SubresourceLoader::didCancel(), right before we crash. We're re-entering the ResourceLoader and finishing it while in the process of cancelling it. Note that the ResourceLoader is not yet freed (it's RefPtr<> protected). It's just accessing members that it already nulled. chrome.dll!WebCore::ResourceLoader::releaseResources() Line 91 C++ chrome.dll!WebCore::ResourceLoader::didFinishLoading(double finishTime=1290207487.7740891) Line 302 + 0xf bytes C++ chrome.dll!WebCore::SubresourceLoader::didFinishLoading(double finishTime=1290207487.7740891) Line 188 C++ chrome.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal=0x04e77c20, double finishTime=1290207487.7740891) Line 435 + 0x18 bytes C++ chrome.dll!WebCore::ResourceHandleInternal::didFinishLoading(WebKit::WebURLLoader * __formal=0x05e232c0, double finishTime=1290207487.7740891) Line 191 + 0x2e bytes C++ chrome.dll!webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest(const URLRequestStatus & status={...}, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & security_info="", const base::Time & completion_time={...}) Line 652 + 0x2c bytes C++ chrome.dll!ResourceDispatcher::OnRequestComplete(int request_id=146, const URLRequestStatus & status={...}, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & security_info="", const base::Time & completion_time={...}) Line 439 + 0x1b bytes C++ chrome.dll!DispatchToMethod<ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::Time const &),int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,base::Time>(ResourceDispatcher * obj=0x01d99aa0, void (int, const URLRequestStatus &, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > &, const base::Time &)* method=0x5c00b460, const Tuple4<int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,base::Time> & arg={...}) Line 573 + 0x23 bytes C++ chrome.dll!IPC::MessageWithTuple<Tuple4<int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,base::Time> >::Dispatch<ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::Time const &)>(const IPC::Message * msg=0x060ae928, ResourceDispatcher * obj=0x01d99aa0, void (int, const URLRequestStatus &, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > &, const base::Time &)* func=0x5c00b460) Line 944 + 0x11 bytes C++ chrome.dll!ResourceDispatcher::DispatchMessageW(const IPC::Message & message={...}) Line 509 + 0x12 bytes C++ chrome.dll!ResourceDispatcher::OnMessageReceived(const IPC::Message & message={...}) Line 297 C++ chrome.dll!ChildThread::OnMessageReceived(const IPC::Message & msg={...}) Line 139 + 0x19 bytes C++ chrome.dll!IPC::ChannelProxy::Context::OnDispatchMessage(const IPC::Message & message={...}) Line 232 + 0x19 bytes C++ chrome.dll!DispatchToMethod<IPC::ChannelProxy::Context,void (__thiscall IPC::ChannelProxy::Context::*)(IPC::Message const &),IPC::Message>(IPC::ChannelProxy::Context * obj=0x01de0000, void (const IPC::Message &)* method=0x5ace84b0, const Tuple1<IPC::Message> & arg={...}) Line 554 + 0xf bytes C++ chrome.dll!RunnableMethod<IPC::ChannelProxy::Context,void (__thiscall IPC::ChannelProxy::Context::*)(IPC::Message const &),Tuple1<IPC::Message> >::Run() Line 330 + 0x1e bytes C++ chrome.dll!MessageLoop::RunTask(Task * task=0x060ae900) Line 418 + 0xf bytes C++ chrome.dll!MessageLoop::DeferOrRunPendingTask(const MessageLoop::PendingTask & pending_task={...}) Line 430 C++ chrome.dll!MessageLoop::DoWork() Line 534 + 0xc bytes C++ chrome.dll!base::MessagePumpDefault::Run(base::MessagePump::Delegate * delegate=0x003ff06c) Line 23 + 0xf bytes C++ chrome.dll!MessageLoop::RunInternal() Line 266 + 0x2a bytes C++ chrome.dll!MessageLoop::RunHandler() Line 239 C++ chrome.dll!MessageLoop::Run() Line 217 C++ chrome.dll!IPC::SyncChannel::WaitForReplyWithNestedMessageLoop(IPC::SyncChannel::SyncContext * context=0x01de0000) Line 476 C++ chrome.dll!IPC::SyncChannel::WaitForReply(IPC::SyncChannel::SyncContext * context=0x01de0000, base::WaitableEvent * pump_messages_event=0x01d90028) Line 442 + 0x9 bytes C++ chrome.dll!IPC::SyncChannel::SendWithTimeout(IPC::Message * message=0x04ee8d70, int timeout_ms=-1) Line 417 + 0x12 bytes C++ chrome.dll!IPC::SyncChannel::Send(IPC::Message * message=0x04ee8d70) Line 381 + 0x15 bytes C++ chrome.dll!ChildThread::Send(IPC::Message * msg=0x04ee8d70) Line 96 + 0x21 bytes C++ chrome.dll!RenderThread::Send(IPC::Message * msg=0x04ee8d70) Line 431 + 0xf bytes C++ chrome.dll!RenderWidget::Send(IPC::Message * message=0x04ee8d70) Line 186 + 0x19 bytes C++ chrome.dll!PrintWebViewHelper::Send(IPC::Message * msg=0x04ee8d70) Line 254 + 0x1d bytes C++ chrome.dll!PrintWebViewHelper::GetPrintSettingsFromUser(WebKit::WebFrame * frame=0x04ebd160, int expected_pages_count=2, bool use_browser_overlays=true) Line 428 + 0xf bytes C++ chrome.dll!PrintWebViewHelper::Print(WebKit::WebFrame * frame=0x04ebd160, bool script_initiated=true, bool is_preview=false) Line 145 + 0x14 bytes C++ chrome.dll!RenderView::Print(WebKit::WebFrame * frame=0x04ebd160, bool script_initiated=true, bool is_preview=false) Line 5185 C++ chrome.dll!RenderView::printPage(WebKit::WebFrame * frame=0x04ebd160) Line 1970 C++ chrome.dll!WebKit::ChromeClientImpl::print(WebCore::Frame * frame=0x05c42800) Line 628 + 0x2a bytes C++ chrome.dll!WebCore::Chrome::print(WebCore::Frame * frame=0x05c42800) Line 418 + 0x1c bytes C++ chrome.dll!WebCore::DOMWindow::print() Line 901 C++ chrome.dll!WebCore::DOMWindow::finishedLoading() Line 1587 C++ chrome.dll!WebCore::DocumentLoader::updateLoading() Line 351 C++ chrome.dll!WebCore::DocumentLoader::removeSubresourceLoader(WebCore::ResourceLoader * loader=0x055d3400) Line 710 C++ chrome.dll!WebCore::SubresourceLoader::didCancel(const WebCore::ResourceError & error={...}) Line 230 C++ chrome.dll!WebCore::ResourceLoader::cancel(const WebCore::ResourceError & error={...}) Line 378 + 0x1f bytes C++ 338481 1 rtenneti 2011-01-21 19:52:34 -0800 Raman Tenneti <rtenneti@google.com> to Adam Barth <abarth@google.com> cc Jim Roskind <jar@google.com> date Fri, Jan 21, 2011 at 2:25 PM subject Re: crash in webcore ResouceLoader mailed-by google.com hide details 2:25 PM (5 hours ago) Hi Adam, If we look at the callstack for the crash, there's usually a correct point to do the null check. Oftentimes that's close to the use, but if something higher on the callstack does't make sense when the pointer is null, we might want to add the check there instead. The above is a good idea. Add a check to MainResourceLoader.cpp to check if m_DoocumentLoader is not null, then only call ResourceLoader::didCancel. Also in ResourceLoader:didCancel, return if m_documentLoader is null. If it not null, then access the m_documentLoader and the next steps: void MainResourceLoader::didCancel(const ResourceError& error) ..... if (m_documentLoader) ResourceLoader::didCancel(error); } void ResourceLoader::didCancel(const ResourceError& error) { ... if (!m_documentLoader) return; m_documentLoader->cancelPendingSubstituteLoad(this); I came across the following is the crash while investigating this bug. It is a different stack trace than the one reported by go/crash. It looks like in the following case we are trying to resume. m_suspended was false and we are asserting it should be true. (I thought I duplicated the bug, but was seeing a different problem). thanks, raman chrome.dll!WebCore::SuspendableTimer::resume() Line 72 + 0x24 bytes C++ chrome.dll!WebCore::ScriptExecutionContext::resumeActiveDOMObjects() Line 203 + 0x1c bytes C++ > chrome.dll!WebCore::PageGroupLoadDeferrer::~PageGroupLoadDeferrer() Line 74 C++ chrome.dll!WebCore::PageGroupLoadDeferrer::`scalar deleting destructor'() + 0x16 bytes C++ chrome.dll!WebKit::WebView::didExitModalLoop() Line 257 + 0x25 bytes C++ chrome.dll!RenderThread::Send(IPC::Message * msg=0x06c49640) Line 424 C++ chrome.dll!RenderWidget::Send(IPC::Message * message=0x06c49640) Line 191 + 0x19 bytes C++ chrome.dll!PrintWebViewHelper::Send(IPC::Message * msg=0x06c49640) Line 283 + 0x1d bytes C++ chrome.dll!PrintWebViewHelper::GetPrintSettingsFromUser(WebKit::WebFrame * frame=0x0628b840, int expected_pages_count=1, bool use_browser_overlays=true) Line 480 + 0xf bytes C++ chrome.dll!PrintWebViewHelper::Print(WebKit::WebFrame * frame=0x0628b840, WebKit::WebNode * node=0x00000000, bool script_initiated=true, bool is_preview=false) Line 167 + 0x15 bytes C++ chrome.dll!PrintWebViewHelper::PrintFrame(WebKit::WebFrame * frame=0x0628b840, bool script_initiated=true, bool is_preview=false) Line 102 C++ chrome.dll!RenderView::Print(WebKit::WebFrame * frame=0x0628b840, bool script_initiated=true, bool is_preview=false) Line 5296 C++ chrome.dll!RenderView::printPage(WebKit::WebFrame * frame=0x0628b840) Line 2161 C++ chrome.dll!WebKit::ChromeClientImpl::print(WebCore::Frame * frame=0x06506200) Line 623 + 0x2a bytes C++ chrome.dll!WebCore::Chrome::print(WebCore::Frame * frame=0x06506200) Line 415 + 0x1c bytes C++ chrome.dll!WebCore::DOMWindow::print() Line 905 C++ chrome.dll!WebCore::DOMWindow::finishedLoading() Line 1587 C++ chrome.dll!WebCore::DocumentLoader::updateLoading() Line 370 C++ chrome.dll!WebCore::DocumentLoader::removeSubresourceLoader(WebCore::ResourceLoader * loader=0x06551000) Line 729 C++ chrome.dll!WebCore::SubresourceLoader::didFinishLoading(double finishTime=1295645962.2659061) Line 188 C++ chrome.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal=0x06c73ad0, double finishTime=1295645962.2659061) Line 439 + 0x18 bytes C++ chrome.dll!WebCore::ResourceHandleInternal::didFinishLoading(WebKit::WebURLLoader * __formal=0x07058008, double finishTime=1295645962.2659061) Line 191 + 0x2e bytes C++ chrome.dll!webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest(const net::URLRequestStatus & status={...}, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & security_info="", const base::Time & completion_time={...}) Line 657 + 0x2c bytes C++ chrome.dll!ResourceDispatcher::OnRequestComplete(int request_id=50, const net::URLRequestStatus & status={...}, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & security_info="", const base::Time & completion_time={...}) Line 457 + 0x1b bytes C++ chrome.dll!DispatchToMethod<ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,net::URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::Time const &),int,net::URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,base::Time>(ResourceDispatcher * obj=0x004e7730, void (int, const net::URLRequestStatus &, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > &, const base::Time &)* method=0x58f7d120, const Tuple4<int,net::URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,base::Time> & arg={...}) Line 570 + 0x23 bytes C++ chrome.dll!IPC::MessageWithTuple<Tuple4<int,net::URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,base::Time> >::Dispatch<ResourceDispatcher,ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,net::URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::Time const &)>(const IPC::Message * msg=0x0703f5a8, ResourceDispatcher * obj=0x004e7730, ResourceDispatcher * sender=0x004e7730, void (int, const net::URLRequestStatus &, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > &, const base::Time &)* func=0x58f7d120) Line 928 + 0x11 bytes C++ chrome.dll!ResourceDispatcher::DispatchMessageW(const IPC::Message & message={...}) Line 530 + 0x16 bytes C++ chrome.dll!ResourceDispatcher::OnMessageReceived(const IPC::Message & message={...}) Line 298 C++ chrome.dll!ChildThread::OnMessageReceived(const IPC::Message & msg={...}) Line 144 + 0x2d bytes C++ chrome.dll!IPC::ChannelProxy::Context::OnDispatchMessage(const IPC::Message & message={...}) Line 255 + 0x19 bytes C++ chrome.dll!DispatchToMethod<IPC::ChannelProxy::Context,void (__thiscall IPC::ChannelProxy::Context::*)(IPC::Message const &),IPC::Message>(IPC::ChannelProxy::Context * obj=0x0052e000, void (const IPC::Message &)* method=0x57b699a0, const Tuple1<IPC::Message> & arg={...}) Line 551 + 0xf bytes C++ chrome.dll!RunnableMethod<IPC::ChannelProxy::Context,void (__thiscall IPC::ChannelProxy::Context::*)(IPC::Message const &),Tuple1<IPC::Message> >::Run() Line 331 + 0x1e bytes C++ chrome.dll!MessageLoop::RunTask(Task * task=0x0703f580) Line 356 + 0xf bytes C++ chrome.dll!MessageLoop::DeferOrRunPendingTask(const MessageLoop::PendingTask & pending_task={...}) Line 368 C++ chrome.dll!MessageLoop::DoWork() Line 558 + 0xc bytes C++ chrome.dll!base::MessagePumpDefault::Run(base::MessagePump::Delegate * delegate=0x0037ed40) Line 23 + 0xf bytes C++ chrome.dll!MessageLoop::RunInternal() Line 331 + 0x2a bytes C++ chrome.dll!MessageLoop::RunHandler() Line 305 C++ chrome.dll!MessageLoop::Run() Line 235 C++ chrome.dll!RendererMain(const MainFunctionParams & parameters={...}) Line 298 C++ chrome.dll!`anonymous namespace'::RunNamedProcessTypeMain(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & process_type="renderer", const MainFunctionParams & main_function_params={...}) Line 593 + 0x12 bytes C++ chrome.dll!ChromeMain(HINSTANCE__ * instance=0x01060000, sandbox::SandboxInterfaceInfo * sandbox_info=0x0037f6d8, wchar_t * command_line_unused=0x003e1d3e) Line 919 + 0x10 bytes C++ chrome.exe!MainDllLoader::Launch(HINSTANCE__ * instance=0x01060000, sandbox::SandboxInterfaceInfo * sbox_info=0x0037f6d8) Line 280 + 0x1d bytes C++ chrome.exe!wWinMain(HINSTANCE__ * instance=0x01060000, HINSTANCE__ * __formal=0x00000000, HINSTANCE__ * __formal=0x00000000, HINSTANCE__ * __formal=0x00000000) Line 46 + 0x10 bytes C++ chrome.exe!__tmainCRTStartup() Line 263 + 0x2c bytes C chrome.exe!wWinMainCRTStartup() Line 182 C kernel32.dll!@BaseThreadInitThunk@12() + 0x12 bytes ntdll.dll!___RtlUserThreadStart@8() + 0x27 bytes ntdll.dll!__RtlUserThreadStart@8() + 0x1b bytes - Show quoted text - 338483 2 rtenneti 2011-01-21 19:56:01 -0800 From Jim Roskind <jar@google.com> date Fri, Jan 21, 2011 at 1:50 PM Interesting.... so if it is all single threaded, and scheduling is the issue... then the thread must (deep down) start to service messages again (effectively yielding to other pending tasks). Is this part of the webkit model? Are tasks allowed to suspend in such a way? Having argued strongly on Chrome *against* nested message loops (places where threads can pause/yield), I'm sad to hear that Webkit would expose such a "feature." Where would a breakpoint need to be placed in webkit to catch such a re-entrant dispatcher? Putting a breakpoint there would prove your point (that re-entrancy was facilitated). It would also make it clear where the null check has to be to ensure that it is after any/all plausible re-entrancy has completed. Jim - Show quoted text - From Adam Barth <abarth@google.com> date Fri, Jan 21, 2011 at 2:52 PM WebKit does run nested message loops because of things like synchronous XMLHttpRequest. You'd see them on the stack though. Much of WebKit needs to be re-entrant because we call out to JavaScript, which can call back into WebCore. Adam chrome.dll!WebKit::WebView::didExitModalLoop() Line 257 + 0x25 bytes C++ That frame is indicative of a nested message loop. We had a modal loop with a bunch of junk on the stack. Adam 338492 3 79822 rtenneti 2011-01-21 20:29:34 -0800 Created attachment 79822 defensive checks for m_documentLoader being NULL in FrameLoader.cpp, we check for m_documentLoader before accessing it. Because the code is reentrant, releaseResources could have been called and m_documentLoader is set to 0. This is a hard bug to duplicate. I was able to duplicate it with the chrome build by cancelling the Print dialog. Wasn't able to reproduce it after adding these checks. thanks, raman 338494 4 webkit.review.bot 2011-01-21 20:32:21 -0800 Attachment 79822 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Source/WebCore/ChangeLog:7: Line contains tab character. [whitespace/tab] [5] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style. 338510 5 79822 abarth 2011-01-21 22:05:20 -0800 Comment on attachment 79822 defensive checks for m_documentLoader being NULL View in context: https://bugs.webkit.org/attachment.cgi?id=79822&action=review > Source/WebCore/ChangeLog:9 > + Covered by Chromium browser_tests. It's much better if we can test changes using only things in webkit.org. That way regressions don't surprise use down the road. We've talked about trying to make a layout test for this change. It's probably worthwhile to explain what you tried here and what trouble you ran into. > Source/WebCore/loader/ResourceLoader.cpp:84 > ASSERT(!m_reachedTerminalState); > + if (m_reachedTerminalState) > + return; Generally we don't have both an assert and code to handle the opposite of the assert. If this case occurs, that means the assert is wrong and should be removed. However, we might try to look at "svn blame" to understand why this assert was added. It's a clue telling us that something else might be wrong. 338546 6 79822 ap 2011-01-22 01:43:47 -0800 Comment on attachment 79822 defensive checks for m_documentLoader being NULL r- for lack of tests and for tabs. This looks very much like some bug I investigated recently, so I'd like to think about this in depth. Generally, early returns on null checks should be added very cautiously, because it's hard to tell which class invariants get violated by not running the remainder of the function, and you end up investigating much more mysterious bugs a few months later. Thanks for posting a lot of detail about your investigation, it's really helpful. 338547 7 ap 2011-01-22 01:50:36 -0800 I found the bug I've been looking into - it's bug 51357. Do you agree that this is a duplicate? 338605 8 rtenneti 2011-01-22 10:50:01 -0800 bug 51357 seems to be similar to this bug. The stack trace in this bug may be slightly different, but the root cause may be the same. I agree it is better to fix the root cause of the problem which will fix different manifestations. Will mark this as a duplicate of https://bugs.webkit.org/show_bug.cgi?id=51357 338606 9 rtenneti 2011-01-22 10:50:22 -0800 *** This bug has been marked as a duplicate of bug 51357 *** 338614 10 rtenneti 2011-01-22 11:23:34 -0800 (fyi) The chrome built with the attached patch didn't crash when I opened the following link, whereas Google Chrome 8.0.552.237 crashes. https://bugs.webkit.org/attachment.cgi?id=77044 thanks, raman 79822 2011-01-21 20:29:34 -0800 2011-01-22 01:43:47 -0800 defensive checks for m_documentLoader being NULL ResourceLoaderCrash.txt text/plain 2302 rtenneti SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDc2NDI0KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTggQEAKKzIwMTEtMDEtMjEgIHJlc291cmNl TG9hZGVyQ3Jhc2ggIDxydGVubmV0aUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ3Jhc2ggaW4gUmVzb3VyY2VMb2FkZXI6OmRp ZENhbmNlbC4gV2UgYXJlIGFzc3VtaW5nIG1fZG9jdW1lbnRMb2FkZXIgYXMgdmFsaWQuCisJQnV0 IHJlbGVhc2VSZXNvdXJjZXMoKSB3YXMgY2FsbGVkIGFuZCBtX2RvY3VtZW50TG9hZGVyIGlzIE5V TEwuIAorCWh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01Mjk0NQorCisg ICAgICAgIENvdmVyZWQgYnkgQ2hyb21pdW0gYnJvd3Nlcl90ZXN0cy4KKworICAgICAgICAqIGxv YWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwOgorICAgICAgICAqIGxvYWRlci9SZXNvdXJjZUxv YWRlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZXNvdXJjZUxvYWRlcjo6cmVsZWFzZVJlc291 cmNlcyk6CisgICAgICAgIChXZWJDb3JlOjpSZXNvdXJjZUxvYWRlcjo6ZGlkQ2FuY2VsKToKKwog MjAxMS0wMS0yMSAgQW5kcmVhcyBLbGluZyAgPGtsaW5nQHdlYmtpdC5vcmc+CiAKICAgICAgICAg UmV2aWV3ZWQgYnkgS2VubmV0aCBSb2hkZSBDaHJpc3RpYW5zZW4uCkluZGV4OiBTb3VyY2UvV2Vi Q29yZS9sb2FkZXIvTWFpblJlc291cmNlTG9hZGVyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViQ29yZS9sb2FkZXIvTWFpblJlc291cmNlTG9hZGVyLmNwcAkocmV2aXNpb24gNzY0MTkpCisr KyBTb3VyY2UvV2ViQ29yZS9sb2FkZXIvTWFpblJlc291cmNlTG9hZGVyLmNwcAkod29ya2luZyBj b3B5KQpAQCAtMTA2LDcgKzEwNiw4IEBAIHZvaWQgTWFpblJlc291cmNlTG9hZGVyOjpkaWRDYW5j ZWwoY29uc3QKICAgICAgICAgZGVyZWYoKTsgLy8gYmFsYW5jZXMgcmVmIGluIGRpZFJlY2VpdmVS ZXNwb25zZQogICAgIH0KICAgICBmcmFtZUxvYWRlcigpLT5yZWNlaXZlZE1haW5SZXNvdXJjZUVy cm9yKGVycm9yLCB0cnVlKTsKLSAgICBSZXNvdXJjZUxvYWRlcjo6ZGlkQ2FuY2VsKGVycm9yKTsK KyAgICBpZiAobV9kb2N1bWVudExvYWRlcikKKyAgICAgICAgUmVzb3VyY2VMb2FkZXI6OmRpZENh bmNlbChlcnJvcik7CiB9CiAKIFJlc291cmNlRXJyb3IgTWFpblJlc291cmNlTG9hZGVyOjppbnRl cnJ1cHRpb25Gb3JQb2xpY3lDaGFuZ2VFcnJvcigpIGNvbnN0CkluZGV4OiBTb3VyY2UvV2ViQ29y ZS9sb2FkZXIvUmVzb3VyY2VMb2FkZXIuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3Jl L2xvYWRlci9SZXNvdXJjZUxvYWRlci5jcHAJKHJldmlzaW9uIDc2NDE5KQorKysgU291cmNlL1dl YkNvcmUvbG9hZGVyL1Jlc291cmNlTG9hZGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtODAsNiAr ODAsOCBAQCBSZXNvdXJjZUxvYWRlcjo6flJlc291cmNlTG9hZGVyKCkKIHZvaWQgUmVzb3VyY2VM b2FkZXI6OnJlbGVhc2VSZXNvdXJjZXMoKQogewogICAgIEFTU0VSVCghbV9yZWFjaGVkVGVybWlu YWxTdGF0ZSk7CisgICAgaWYgKG1fcmVhY2hlZFRlcm1pbmFsU3RhdGUpCisgICAgICAgIHJldHVy bjsKICAgICAKICAgICAvLyBJdCdzIHBvc3NpYmxlIHRoYXQgd2hlbiB3ZSByZWxlYXNlIHRoZSBo YW5kbGUsIGl0IHdpbGwgYmUKICAgICAvLyBkZWFsbG9jYXRlZCBhbmQgcmVsZWFzZSB0aGUgbGFz dCByZWZlcmVuY2UgdG8gdGhpcyBvYmplY3QuCkBAIC0zNTMsNiArMzU1LDkgQEAgdm9pZCBSZXNv dXJjZUxvYWRlcjo6ZGlkQ2FuY2VsKGNvbnN0IFJlcwogICAgIGlmIChtX2hhbmRsZSkKICAgICAg ICAgbV9oYW5kbGUtPmNsZWFyQXV0aGVudGljYXRpb24oKTsKIAorICAgIGlmICghbV9kb2N1bWVu dExvYWRlcikKKyAgICAgICAgcmV0dXJuOworCiAgICAgbV9kb2N1bWVudExvYWRlci0+Y2FuY2Vs UGVuZGluZ1N1YnN0aXR1dGVMb2FkKHRoaXMpOwogICAgIGlmIChtX2hhbmRsZSkgewogICAgICAg ICBtX2hhbmRsZS0+Y2FuY2VsKCk7Cg==