New issue
Advanced search Search tips

Issue 872944 link

Starred by 1 user
Status: Fixed
Owner:
yuweih@chromium.org
Closed: Aug 14
Cc:
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

[CRD iOS] Thread issue with RendererProxy and QueuedTaskPoster

Project Member Reported by yuweih@chromium.org, Aug 9 
Currently RendererProxy is:
1. Created on the UI thread
2. Initialized on the display thread, which binds its WeakPtrFactory to the display thread
3. Used entirely on the UI thread
4. Deleted on the display thread because of #2

QueuedTaskPoster is used entirely on the UI thread, but #4 above forces it to be deleted on the display thread. QueuedTaskPoster has a WeakPtrFactory which is bound to the UI thread so technically speaking it can't be deleted on the display thread. The WeakPtrFactory's dtor should have failed a thread checker's DCHECK but for some reason it rarely happens on our app, though we still need to fix this issue to prevent thread-related crashes.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 13 

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

commit 3915a08ac269960f617d8e9e476daf3b0bb7331c
Author: Yuwei Huang <yuweih@chromium.org>
Date: Mon Aug 13 19:48:50 2018

[CRD iOS] Fix thread issue of RendererProxy and QueuedTaskPoster

RendererProxy is used mainly on the UI thread, but it is deleted on the
display thread because GlDisplayRenderer binds its WeakPtrFactory to
the display thread when initializing it. This forces QueuedTaskPoster
to be deleted on the wrong thread, which may be a source of crash. You
can see more details in the bug.

This CL resolves this by making GlDisplayRenderer::Core initialize
RendererProxy on the UI thread.

Bug:  872944 
Change-Id: I37dbd20aad80dd05e4eb0ad410c339fdfa5053c5
Reviewed-on: https://chromium-review.googlesource.com/1170133
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582674}
[modify] https://crrev.com/3915a08ac269960f617d8e9e476daf3b0bb7331c/remoting/client/display/renderer_proxy.cc
[modify] https://crrev.com/3915a08ac269960f617d8e9e476daf3b0bb7331c/remoting/client/display/renderer_proxy.h
[modify] https://crrev.com/3915a08ac269960f617d8e9e476daf3b0bb7331c/remoting/client/queued_task_poster.cc
[modify] https://crrev.com/3915a08ac269960f617d8e9e476daf3b0bb7331c/remoting/ios/display/gl_display_handler.h
[modify] https://crrev.com/3915a08ac269960f617d8e9e476daf3b0bb7331c/remoting/ios/display/gl_display_handler.mm
[modify] https://crrev.com/3915a08ac269960f617d8e9e476daf3b0bb7331c/remoting/ios/session/remoting_client.mm

Comment 2 by yuweih@chromium.org, Aug 14

Status: Fixed (was: Assigned)

Comment 3 by yuweih@chromium.org, Oct 2

Labels: M-70
►

Sign in to add a comment