breakpoints affect setTimeout callback execution order
Reported by
gkub...@gmail.com,
Aug 16 2016
|
|||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36 Steps to reproduce the problem: 1. Go to https://jsfiddle.net/gkubisa/werodo4x/ 2. Open Dev Tools 3. Run the fiddle 4. Continue execution on each breakpoint What is the expected behavior? The setTimeout callbacks should be executed in the order in which they were registered. The following should be printed to the console: first first first first second What went wrong? Some setTimeout callbacks were executed out of order. For example, the following might be printed to the console: first first second first first Did this work before? N/A Chrome version: 52.0.2743.116 Channel: stable OS Version: Ubuntu 15.10 Flash Version: Shockwave Flash 22.0 r0 The setTimeout callback execution order seems to be random and is incorrect most of the time. Rerun the script, if the issue is not reproduced the first time. Here's the code I used to reproduce this issue, in case jsfiddle.net is not available: for (let i = 0; i < 4; ++i) { debugger; setTimeout(function() { debugger; console.log('first'); }, 0) } debugger; setTimeout(function() { debugger; console.log('second'); }, 0);
,
Aug 22 2016
This might have something to do with disabling and enabling timers around breakpoints.
,
Aug 22 2016
,
Oct 16 2017
,
Oct 4
Sami, looks like your intuition was right :) When pausing and later unpausing timers at breakpoints, ContextLifecycleNotifier::NotifySuspending/ResumingPausableObjects() [1] iterates over an unordered HeapHashSet [2]. Thus, timers with the same "remaining" time may end up being rescheduled in a different order when unpausing. Sounds like the fix would be to use a HeapLinkedHashSet instead. [1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/context_lifecycle_notifier.cc?l=35 [2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/lifecycle_notifier.h?gsn=ForEachObserver&l=85
,
Oct 4
I'll try this and see if it helps.
,
Oct 4
,
Oct 4
Yup, that helps, patch is here: https://chromium-review.googlesource.com/c/chromium/src/+/1261182. Not sure if this change will have an impact on performance elsewhere. I'd be happy to try landing this and reverting if it turns out to be too bad.
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5dab35524b66776bcdec032b5554a03c2d9b7a4e commit 5dab35524b66776bcdec032b5554a03c2d9b7a4e Author: Eric Seckler <eseckler@chromium.org> Date: Thu Oct 04 12:15:13 2018 blink: Preserve order in Lifecycle observer iteration This fixes a bug where the order of scheduled timeouts is reshuffled during debugging via DevTools, since these timeouts are paused and unpaused when a breakpoint is reached, and notified about this via a Lifecycle observer. I'd like to try replacing the HeapHashSet currently used by Lifecycle observers with a HeapLinkedHashSet to preserve the observer iteration order, resolving the problem above. If this turns out to negatively impact performance elsewhere significantly, we can revert. Bug: 638255 Change-Id: I5c0be5c2132d3869863e3be6b8913c1f3ea3859f Reviewed-on: https://chromium-review.googlesource.com/c/1261182 Reviewed-by: Alexander Timin <altimin@chromium.org> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org> Commit-Queue: Eric Seckler <eseckler@chromium.org> Cr-Commit-Position: refs/heads/master@{#596619} [modify] https://crrev.com/5dab35524b66776bcdec032b5554a03c2d9b7a4e/third_party/blink/renderer/platform/lifecycle_notifier.h
,
Oct 4
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by l...@chromium.org
, Aug 16 2016Status: Assigned (was: Unconfirmed)