New issue
Advanced search Search tips

Issue 754360 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK failure in ScriptRunner::DoTryStream()

Project Member Reported by thildebr@chromium.org, Aug 10 2017

Issue description

Receive Aw, Snap! page when loading buzzfeed.com/tasty.

Logs are showing:
[FATAL:ScriptRunner.cpp(287)] Check failed: success == pending_script->IsCurrentlyStreaming() (0 vs. 1)

Android version 7.1.2
Chrome version 62.0.3182.0
 
Labels: OS-Linux
Receiving this on Linux desktop as well. ToT loading google.

[1:1:0811/105950.398182:FATAL:ScriptRunner.cpp(287)] Check failed: success == pending_script->IsCurrentlyStreaming() (0 vs. 1)
#0 0x7f5a557e2947 base::debug::StackTrace::StackTrace()
#1 0x7f5a558098c1 logging::LogMessage::~LogMessage()
#2 0x7f5a4e3a3000 blink::ScriptRunner::DoTryStream()
#3 0x7f5a4e3a24a8 blink::ScriptRunner::TryStreamAny()
#4 0x7f5a4e3a2745 blink::ScriptRunner::NotifyScriptStreamerFinished()
#5 0x7f5a4e2b69e0 blink::ClassicPendingScript::StreamingFinished()
#6 0x7f5a4dfb3b32 blink::ScriptStreamer::NotifyFinishedToClient()
#7 0x7f5a4dfb3243 blink::ScriptStreamer::NotifyFinished()
#8 0x7f5a4e2b6fc9 blink::ClassicPendingScript::NotifyFinished()
#9 0x7f5a4d4f22a7 blink::Resource::CheckNotify()
#10 0x7f5a4d4f34e1 blink::Resource::Finish()
#11 0x7f5a4d503710 blink::ResourceFetcher::HandleLoaderFinish()
#12 0x7f5a52879b8a content::WebURLLoaderImpl::Context::OnCompletedRequest()
#13 0x7f5a5284f1f6 content::ResourceDispatcher::OnRequestComplete()
#14 0x7f5a5286f023 content::URLResponseBodyConsumer::OnReadable()
#15 0x7f5a527db170 mojo::SimpleWatcher::DiscardReadyState()
#16 0x7f5a54a541e2 mojo::SimpleWatcher::OnHandleReady()
#17 0x7f5a54a54788 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#18 0x7f5a557e318b base::debug::TaskAnnotator::RunTask()
#19 0x7f5a4d528e48 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
#20 0x7f5a4d526944 blink::scheduler::TaskQueueManager::DoWork()
#21 0x7f5a4d52ae22 _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler16TaskQueueManagerEFvbEJNS_7WeakPtrIS5_EEbEEEFvvEE3RunEPNS0_13BindStateBaseE
#22 0x7f5a557e318b base::debug::TaskAnnotator::RunTask()
#23 0x7f5a5581621d base::MessageLoop::RunTask()
#24 0x7f5a5581655b base::MessageLoop::DeferOrRunPendingTask()
#25 0x7f5a558168c4 base::MessageLoop::DoWork()
#26 0x7f5a55818200 base::MessagePumpDefault::Run()
#27 0x7f5a55815c2a base::MessageLoop::Run()
#28 0x7f5a55849a67 base::RunLoop::Run()
#29 0x7f5a5334ff5f content::RendererMain()
#30 0x7f5a534bc449 content::RunZygote()
#31 0x7f5a534bcd3a content::RunNamedProcessTypeMain()
#32 0x7f5a534bd690 content::ContentMainRunnerImpl::Run()
#33 0x7f5a55d2ae23 service_manager::Main()
#34 0x7f5a534bc0f2 content::ContentMain()
#35 0x563d357b23cc ChromeMain
#36 0x7f5a49069f45 __libc_start_main
#37 0x563d357b221f <unknown>

Comment 2 by hayato@chromium.org, Aug 18 2017

Components: -Blink>DOM Blink
Status: Assigned (was: Untriaged)

Comment 3 by tkent@chromium.org, Aug 18 2017

Components: -Blink Blink>HTML>Script
Cc: hirosh...@chromium.org
Summary: DCHECK failure in ScriptRunner::DoTryStream() (was: ScriptRunner.cpp DCHECK failure)
I can't reproduce this.

50 runs on tip-of-tree, URL from #0, Chromium Linux debug (per #1), did not trigger the assertion once.

That gave me hope it might have been fixed on tip-of-tree already (there are several refactorings ongoing in that general area), so I went back to 62.0.3182.0 (commit 928ca066bbe4e9a1111e014d6cc99ec9f3638466). 50 runs of that version didn't reproduce it either.

So... I don't know how to repro this.


thildebr, dtapuska: Any additional info on how to reproduce this?
This is still reproducing today for me.

ToT is: commit 8f6dd43d6c9f5b420e6cca59ecf829079acd5480

My homepage is set to https://moma.corp.google.com/moma. While that is loading click the URL bar. 

I'm using a release build with dcheck always on.
[Summary of brief discussion w/ dtapuska, for posterity, and for anyone else following along:]

- I [vogelheim] can't reproduce. Suspicion that it's flaky.
- Dave can. Reports that it reproduces quite reliably; both at the given commit plus at tip-of-tree.
- So, "flaky" is wrong.
- We compare build version; gn args; OS.
- Minor difference in gn args. I'll try the exact args to be sure, but that doesn't seem like a likely cause.
- Dave tries with clean profile [ --user-data-dir=$(mktemp -d) ], no longer reproduces.
- So, apparently something to do with whether things are in cache or not.
- Which also explains why I can't reproduce, since I probably have different cache contents.


Interpretation / Best Guess:

The "success" var that goes into the DCHECK does - in some branches - explicitly check whether (some aspect of) loading has finished; but there's no such check in others. That could explain the observed behaviour, because on a proper load from net the finishing would likely take a while, while for a cached resource it might occur more quickly. Also, if this depends on the on-disk cache content, then this timing different would be very persistent.

So... I'm still guessing, but I think I have a lead.


Extra thanks for dtapuska@ for his patience!
 Issue 763016  has been merged into this issue.
 Issue 763016  provides another URL.
Hrmm. I wonder if I'm overlooking something super obvious here...


Both this stack trace & the one from 763016 contain
  TryStreamAny -> DoTryStream.

That code path is guarded by RuntimeEnabledFeatures::WorkStealingInScriptRunnerEnabled, which... is marked experimental, and which I had therefore expected to be off.

@dtapuska: Do you have "experimental web platform features" enabled, by any chance? (https://www.chromium.org/blink/runtime-enabled-features)


---------------

Of course I still need to fix this ASAP, since buggy code isn't ok just because it's behind a flag. But it would explain why I can't repro anything.
I have it enabled on mine, which has the crash.
It turns out, when I enable the feature I can reproduce the bug quite easily with any of the URLs provided here.

\o/ \o/ \o/

Update: I have a tentative fix for this.

But since this particular area of code has been subtle and rather bug prone, I'll spend a bit more time trying to make sure it actually, really, truly fixes things before landing this. So I need to ask for a bit more patience.
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22fa13f20883849ace414b9e79bfdac4037b8984

commit 22fa13f20883849ace414b9e79bfdac4037b8984
Author: Daniel Vogelheim <vogelheim@chromium.org>
Date: Thu Oct 05 10:01:23 2017

Fix spurious DCHECK when WorkStealingInScriptRunner is enabled.

Bug:  754360 
Change-Id: I8dd792787dcf5835eeb279f5ce535cb871f6d1e0
Reviewed-on: https://chromium-review.googlesource.com/660698
Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506691}
[modify] https://crrev.com/22fa13f20883849ace414b9e79bfdac4037b8984/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
[modify] https://crrev.com/22fa13f20883849ace414b9e79bfdac4037b8984/third_party/WebKit/Source/core/dom/ClassicPendingScript.h
[modify] https://crrev.com/22fa13f20883849ace414b9e79bfdac4037b8984/third_party/WebKit/Source/core/dom/ScriptRunner.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/763320544a76d40b9994a01110e96a83c3da8b12

commit 763320544a76d40b9994a01110e96a83c3da8b12
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Thu Oct 05 12:58:03 2017

Revert "Fix spurious DCHECK when WorkStealingInScriptRunner is enabled."

This reverts commit 22fa13f20883849ace414b9e79bfdac4037b8984.

Reason for revert: interactive_ui_tests are timing out. Speculative revert (the other CL in the regression range looks like an unlikely culprit to me).

https://build.chromium.org/p/chromium.win/builders/Win%207%20Tests%20x64%20%281%29/builds/29628

Original change's description:
> Fix spurious DCHECK when WorkStealingInScriptRunner is enabled.
> 
> Bug:  754360 
> Change-Id: I8dd792787dcf5835eeb279f5ce535cb871f6d1e0
> Reviewed-on: https://chromium-review.googlesource.com/660698
> Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
> Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506691}

TBR=hiroshige@chromium.org,kouhei@chromium.org,vogelheim@chromium.org,tzik@chromium.org

Change-Id: Ifad44699988aee7f1a8001897e75b592c7d19ebb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  754360 
Reviewed-on: https://chromium-review.googlesource.com/702336
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506706}
[modify] https://crrev.com/763320544a76d40b9994a01110e96a83c3da8b12/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
[modify] https://crrev.com/763320544a76d40b9994a01110e96a83c3da8b12/third_party/WebKit/Source/core/dom/ClassicPendingScript.h
[modify] https://crrev.com/763320544a76d40b9994a01110e96a83c3da8b12/third_party/WebKit/Source/core/dom/ScriptRunner.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebb8d18f4466a4e17b36174a5a1208c745a365de

commit ebb8d18f4466a4e17b36174a5a1208c745a365de
Author: Daniel Vogelheim <vogelheim@chromium.org>
Date: Thu Oct 05 14:04:41 2017

Revert "Revert "Fix spurious DCHECK when WorkStealingInScriptRunner is enabled.""

This reverts commit 763320544a76d40b9994a01110e96a83c3da8b12.

Reason for revert: Original problem was fixed on tree even before the revert landed; so apparently it wasn't this CL.

Original change's description:
> Revert "Fix spurious DCHECK when WorkStealingInScriptRunner is enabled."
> 
> This reverts commit 22fa13f20883849ace414b9e79bfdac4037b8984.
> 
> Reason for revert: interactive_ui_tests are timing out. Speculative revert (the other CL in the regression range looks like an unlikely culprit to me).
> 
> https://build.chromium.org/p/chromium.win/builders/Win%207%20Tests%20x64%20%281%29/builds/29628
> 
> Original change's description:
> > Fix spurious DCHECK when WorkStealingInScriptRunner is enabled.
> > 
> > Bug:  754360 
> > Change-Id: I8dd792787dcf5835eeb279f5ce535cb871f6d1e0
> > Reviewed-on: https://chromium-review.googlesource.com/660698
> > Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
> > Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
> > Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> > Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#506691}
> 
> TBR=hiroshige@chromium.org,kouhei@chromium.org,vogelheim@chromium.org,tzik@chromium.org
> 
> Change-Id: Ifad44699988aee7f1a8001897e75b592c7d19ebb
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  754360 
> Reviewed-on: https://chromium-review.googlesource.com/702336
> Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
> Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506706}

TBR=tnagel@chromium.org,hiroshige@chromium.org,kouhei@chromium.org,vogelheim@chromium.org,tzik@chromium.org

Change-Id: I35841f0e4a5297fc98c5b11e60bb93399005165c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  754360 
Reviewed-on: https://chromium-review.googlesource.com/702337
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506718}
[modify] https://crrev.com/ebb8d18f4466a4e17b36174a5a1208c745a365de/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
[modify] https://crrev.com/ebb8d18f4466a4e17b36174a5a1208c745a365de/third_party/WebKit/Source/core/dom/ClassicPendingScript.h
[modify] https://crrev.com/ebb8d18f4466a4e17b36174a5a1208c745a365de/third_party/WebKit/Source/core/dom/ScriptRunner.cpp

Cc: vogelheim@chromium.org
 Issue 771092  has been merged into this issue.
Status: Fixed (was: Assigned)

Sign in to add a comment