New issue
Advanced search Search tips

Issue 902653 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

CompletionEvent::Wait shouldn't allow base sync primitives

Project Member Reported by etiennep@chromium.org, Nov 7

Issue description

CompletionEvent::Wait is instantiating it's own ScopedAllowBaseSyncPrimitivesOutsideBlockingScope.
This blindly allow anyone using this class to block.
Instead, the callers would need to provide the allowance.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 8

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

commit 7575b70d508df70d39c99cecd62b9d8fdadff6fe
Author: Etienne Pierre-doray <etiennep@chromium.org>
Date: Thu Nov 08 16:46:33 2018

[TaskScheduler]: Migrate off of ScopedAllowWait in /cc

base::ThreadRestrictions::ScopedAllowWait is deprecated in favor of its more
explicit counterpart.

It should have been replaced by :
 * base::ScopedAllowBaseSyncPrimitivesForTesting in test files.
 * base::ScopedAllowSyncPrimitives in non-test files
 * base::ScopedAllowSyncPrimitivesOutsideBlockingScope when it's used on threads
   that don't allow blocking
The last one is strongly frowned upon but this CL aims to document existing
behavior rather than address it. Owners are encouraged to follow-up by fixing
unnecessary waits and more particularly unnecessary waits
outside-blocking-scope.

Note: The non-for-testing versions require friend'ing in thread_restrictions.h
but care was taken to add these friends ahead of git cl split (since it wasn't
possible to do a line-by-line associated CL split).
Refer to the top-level CL if necessary :
https://chromium-review.googlesource.com/c/chromium/src/+/1288533

Please CQ if LGTY!

This CL was uploaded by git cl split.

R=kylechar@chromium.org

Bug: 766678, 902653
Change-Id: I30a5956028c8d7bca56be07433b2b2f31592ae9f
Reviewed-on: https://chromium-review.googlesource.com/c/1324395
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606497}
[modify] https://crrev.com/7575b70d508df70d39c99cecd62b9d8fdadff6fe/cc/base/completion_event.h
[modify] https://crrev.com/7575b70d508df70d39c99cecd62b9d8fdadff6fe/cc/raster/single_thread_task_graph_runner.cc

Cc: gab@chromium.org etiennep@chromium.org
Owner: sunn...@chromium.org

Sign in to add a comment