[blink scheduler] Fix crashes in (experimental) worker throttling [chromium/src : main]
0 views
Skip to first unread message
Scott Haseley (Gerrit)
Mar 26, 2026, 11:47:31 AM (7 days ago) Mar 26
to Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, chikamu...@chromium.org, kinuko...@chromium.org, scheduler-...@chromium.org
Attention needed from Etienne Pierre-Doray
Scott Haseley added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Scott Haseley . unresolved
Hi Etienne, do you mind reviewing?
Also, I'm curious if worker throttling something your team would be interested in pursuing? I don't know why this work stalled, but seems like it would be nice to finish, even if just to close the loophole of bouncing through a worker to bypass main thread throttling.
File third_party/blink/renderer/platform/scheduler/worker/worker_scheduler_impl_unittest.cc
Line 231, Patchset 1 (Latest):
scheduler_->OnLifecycleStateChanged(SchedulingLifecycleState::kHidden);Scott Haseley . resolved
This used to crash.
Related details
Attention is currently required from:
- Etienne Pierre-Doray
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Mar 26, 2026, 1:07:29 PM (7 days ago) Mar 26
to Scott Haseley, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, chikamu...@chromium.org, kinuko...@chromium.org, scheduler-...@chromium.org
Attention needed from Scott Haseley
Etienne Pierre-Doray voted and added 1 comment![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Scott Haseley
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Scott Haseley (Gerrit)
Mar 26, 2026, 2:16:46 PM (7 days ago) Mar 26
to Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, chikamu...@chromium.org, kinuko...@chromium.org, scheduler-...@chromium.org
Scott Haseley voted and added 2 comments![]( "Open in Gerrit")
Votes added by Scott Haseley
| Commit-Queue | +2 |
2 comments
Patchset-level comments
Scott Haseley . resolved
ty!
Scott Haseley . resolved
Hi Etienne, do you mind reviewing?
Also, I'm curious if worker throttling something your team would be interested in pursuing? I don't know why this work stalled, but seems like it would be nice to finish, even if just to close the loophole of bouncing through a worker to bypass main thread throttling.
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Mar 26, 2026, 2:19:43 PM (7 days ago) Mar 26
to Scott Haseley, Etienne Pierre-Doray, chromium...@chromium.org, blink-...@chromium.org, chikamu...@chromium.org, kinuko...@chromium.org, scheduler-...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[blink scheduler] Fix crashes in (experimental) worker throttling
This CL fixes two potential crashes that occur with
kDedicatedWorkerThrottling** enabled.
1. It's possible for a lifecycle-changing task (e.g. throttling state
change) to run after the worker has been disposed, in which case
the throttleable task queue's destroyed throttler gets dereferenced.
This can occur, for example, if the work task running while the
lifecycle task gets queued calls `self.close()`. This CL fixes this
by checking the disposed state in OnLifecycleStateChanged().
2. The throttling code doesn't handle all `SchedulingLifecycleState`s,
rather it unconditionally decrements the throttling ref count if
transitioning to any state other than kThrottled. This trips a
DCHECK if, for example, transitioning from kNotThrottled to
kHidden since we attempt to decrement the count below zero.
This CL fixes this by checking the previous throttling state and
only manipulating the ref count if transitioning to or from
kThrottled. Note that this feature does not throttle at all in
kHidden, which differs from main thread (nor does it handle
intensive throttling), but this is beyond this scope of this CL.
**kDedicatedWorkerThrottling: this feature has not been in active
development since 2021. Rather than remove this now (since I think we
should consider reviving it), this CL is scoped to fixing some known
crashes.
Bug: 493222148, 40545662
Change-Id: I47a92a89239b164403894c99d11159b93a088b3e
Reviewed-by: Etienne Pierre-Doray <etie...@chromium.org>
Commit-Queue: Scott Haseley <shas...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1605674}
Files:
- M third_party/blink/renderer/platform/scheduler/worker/non_main_thread_task_queue.cc
- M third_party/blink/renderer/platform/scheduler/worker/worker_scheduler_impl.cc
- M third_party/blink/renderer/platform/scheduler/worker/worker_scheduler_impl_unittest.cc
Change size: S
Delta: 3 files changed, 34 insertions(+), 3 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Etienne Pierre-Doray
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages