Fix potential shutdown deadlock in UI service

Fix potential shutdown deadlock in UI service

If the GpuPtr is disconnected while there are pending buffer allocations
in flight, this can lead to the main thread deadlocking on a WaitableEvent
that never signals.

Fixes that by tracking pending allocations and ensuring they're always
signalled on connection error or other shutdown events.

BUG=
R=sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2651443002
Cr-Commit-Position: refs/heads/master@{#445434}
Committed: https://chromium.googlesource.com/chromium/src/+/d0e46de3ff797f53376fa63297a5d41241568fdf

[Ken Rockot(use gerrit already), 2017-01-21 01:09:42]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-21 01:09:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2017-01-21 01:35:07]

Noticed this when trying to land a large service manager refactor. Might need
some cleanup, but WDYT?

[sadrul, 2017-01-21 02:54:31]

lgtm. Thanks!

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
File services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.cc (right):

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.cc:141: if
(gpu_.is_bound()) {
early return if not bound?

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
File services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.h (right):

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.h:51: void
OnGpuMemoryBufferAllocatedOnThread(
Move these above to group with
TearDownThread()/AllocateGpuMemoryBufferOnThread()?

[commit-bot: I haz the power, 2017-01-21 03:46:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-21 03:46:53]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2017-01-23 18:00:23]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[Ken Rockot(use gerrit already), 2017-01-23 18:00:50]

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
File services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.cc (right):

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.cc:141: if
(gpu_.is_bound()) {
On 2017/01/21 at 02:54:31, sadrul wrote:
> early return if not bound?

Done

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
File services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.h (right):

https://codereview.chromium.org/2651443002/diff/1/services/ui/public/cpp/gpu/...
services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.h:51: void
OnGpuMemoryBufferAllocatedOnThread(
On 2017/01/21 at 02:54:31, sadrul wrote:
> Move these above to group with
TearDownThread()/AllocateGpuMemoryBufferOnThread()?

Done

[Ken Rockot(use gerrit already), 2017-01-23 18:00:53]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2017-01-23 18:00:54]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2017-01-23 18:00:55]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2651443002/#ps20001
(title: ".")

[commit-bot: I haz the power, 2017-01-23 18:00:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 19:37:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485194454634560,
"parent_rev": "717012126c57dfe596b6562f9ad17f2a7e537a5b", "commit_rev":
"d0e46de3ff797f53376fa63297a5d41241568fdf"}

[commit-bot: I haz the power, 2017-01-23 19:39:03]

Description was changed from

==========
Fix potential shutdown deadlock in UI service

If the GpuPtr is disconnected while there are pending buffer allocations
in flight, this can lead to the main thread deadlocking on a WaitableEvent
that never signals.

Fixes that by tracking pending allocations and ensuring they're always
signalled on connection error or other shutdown events.

BUG=
R=sadrul@chromium.org
==========

to

==========
Fix potential shutdown deadlock in UI service

If the GpuPtr is disconnected while there are pending buffer allocations
in flight, this can lead to the main thread deadlocking on a WaitableEvent
that never signals.

Fixes that by tracking pending allocations and ensuring they're always
signalled on connection error or other shutdown events.

BUG=
R=sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2651443002
Cr-Commit-Position: refs/heads/master@{#445434}
Committed:
https://chromium.googlesource.com/chromium/src/+/d0e46de3ff797f53376fa63297a5...
==========

[commit-bot: I haz the power, 2017-01-23 19:39:04]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d0e46de3ff797f53376fa63297a5...

[mastiz, 2017-01-25 14:49:20]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2658603004/ by mastiz@chromium.org.

The reason for reverting is: Bisect by tzik@ suggests it could be the culprit
for tsan builder failing, e.g.
https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20TSan%....