DCHECK failure in ScriptRunner::DoTryStream() |
|||||
Issue descriptionReceive 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
,
Aug 18 2017
,
Aug 18 2017
,
Aug 18 2017
,
Aug 29 2017
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?
,
Sep 1 2017
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.
,
Sep 6 2017
[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!
,
Sep 7 2017
Issue 763016 has been merged into this issue.
,
Sep 7 2017
Issue 763016 provides another URL.
,
Sep 8 2017
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.
,
Sep 8 2017
I have it enabled on mine, which has the crash.
,
Sep 8 2017
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/
,
Sep 11 2017
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.
,
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
,
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
,
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
,
Oct 23 2017
,
Oct 23 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dtapu...@chromium.org
, Aug 11 2017