New issue
Advanced search Search tips

Issue 794293 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Thread safety issue with ClientDiscardableManager

Project Member Reported by ericrk@chromium.org, Dec 12 2017

Issue description

ClientDiscardableTextureManager and ClientTransferCache are called from ContextSupport without the context lock held. We need locking in these cases to prevent data races which are currently possible.
 

Comment 1 by ericrk@chromium.org, Dec 12 2017

Description: Show this description

Comment 2 by ericrk@chromium.org, Dec 12 2017

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 13 2017

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

commit 5a20a59933d43be36e46b7ea0d5b76f23909f609
Author: Eric Karl <ericrk@chromium.org>
Date: Wed Dec 13 02:53:16 2017

Add locking to ClientDiscardableTextureManager and ClientTransferCache

ClientDiscardableTextureManager and ClientTransferCache are called from special Threadsafe* functions on ContextSupport without the context lock held. We need locking in these cases to prevent data races which are currently possible.

Bug:  794293 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I70c5a23dc2bd55cb77fb29c3c9057e8c37211e8c
Reviewed-on: https://chromium-review.googlesource.com/823232
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523664}
[modify] https://crrev.com/5a20a59933d43be36e46b7ea0d5b76f23909f609/gpu/command_buffer/client/client_discardable_texture_manager.cc
[modify] https://crrev.com/5a20a59933d43be36e46b7ea0d5b76f23909f609/gpu/command_buffer/client/client_discardable_texture_manager.h
[modify] https://crrev.com/5a20a59933d43be36e46b7ea0d5b76f23909f609/gpu/command_buffer/client/client_transfer_cache.cc
[modify] https://crrev.com/5a20a59933d43be36e46b7ea0d5b76f23909f609/gpu/command_buffer/client/client_transfer_cache.h

Comment 4 by ericrk@chromium.org, Dec 15 2017

Labels: Merge-Request-64
Project Member

Comment 5 by sheriffbot@chromium.org, Dec 15 2017

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by ericrk@chromium.org, Dec 15 2017

We'd like to take this merge to M64 - this is a straightforward change which adds locking to prevent races during GPU image handling. Without this change, we will likely
see rare crashes due to this race.
Labels: -Merge-Review-64 Merge-Approved-64
Approving merge to M64 Chrome OS to avoid the crash scenarios...
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 18 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66423209e74c11b4bb0ec326f7ddbc33e9a1a038

commit 66423209e74c11b4bb0ec326f7ddbc33e9a1a038
Author: Eric Karl <ericrk@chromium.org>
Date: Mon Dec 18 21:51:06 2017

Add locking to ClientDiscardableTextureManager and ClientTransferCache

ClientDiscardableTextureManager and ClientTransferCache are called from special Threadsafe* functions on ContextSupport without the context lock held. We need locking in these cases to prevent data races which are currently possible.

TBR=ericrk@chromium.org

(cherry picked from commit 5a20a59933d43be36e46b7ea0d5b76f23909f609)

Bug:  794293 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I70c5a23dc2bd55cb77fb29c3c9057e8c37211e8c
Reviewed-on: https://chromium-review.googlesource.com/823232
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#523664}
Reviewed-on: https://chromium-review.googlesource.com/832920
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#275}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/66423209e74c11b4bb0ec326f7ddbc33e9a1a038/gpu/command_buffer/client/client_discardable_texture_manager.cc
[modify] https://crrev.com/66423209e74c11b4bb0ec326f7ddbc33e9a1a038/gpu/command_buffer/client/client_discardable_texture_manager.h
[modify] https://crrev.com/66423209e74c11b4bb0ec326f7ddbc33e9a1a038/gpu/command_buffer/client/client_transfer_cache.cc
[modify] https://crrev.com/66423209e74c11b4bb0ec326f7ddbc33e9a1a038/gpu/command_buffer/client/client_transfer_cache.h

Comment 9 by ericrk@chromium.org, Dec 19 2017

Status: Fixed (was: Started)

Sign in to add a comment