New issue
Advanced search Search tips

Issue 644924 link

Starred by 1 user
Status: Verified
Owner:
yuweih@chromium.org
Closed: Sep 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[Remoting Android] OnPixelTransformationChanged Flakiness

Project Member Reported by yuweih@chromium.org, Sep 7 2016 
In some rare circumstance (<5% connection attempt), the app may crash right after connection started giving this tombstone:

```

09-07 15:39:37.377: A/DEBUG(26099):     #00 pc 000475e6  /system/lib/libc.so (pthread_mutex_lock+1)
09-07 15:39:37.378: A/DEBUG(26099):     #01 pc 000b6627  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base8internal8LockImpl4LockEv+22)
09-07 15:39:37.378: A/DEBUG(26099):     #02 pc 000c98c3  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZNK4base17ThreadCheckerImpl19CalledOnValidThreadEv+16)
09-07 15:39:37.378: A/DEBUG(26099):     #03 pc 000499c7  /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.cr.so
09-07 15:39:37.378: A/DEBUG(26099):     #04 pc 0004b627  /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.cr.so
09-07 15:39:37.378: A/DEBUG(26099):     #05 pc 00233259  /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.cr.so
09-07 15:39:37.378: A/DEBUG(26099):     #06 pc 0007a045  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base5debug13TaskAnnotator7RunTaskEPKcRKNS_11PendingTaskE+124)
09-07 15:39:37.378: A/DEBUG(26099):     #07 pc 00091045  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base11MessageLoop7RunTaskERKNS_11PendingTaskE+200)
09-07 15:39:37.378: A/DEBUG(26099):     #08 pc 000911e7  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base11MessageLoop21DeferOrRunPendingTaskENS_11PendingTaskE+20)
09-07 15:39:37.378: A/DEBUG(26099):     #09 pc 00091365  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base11MessageLoop6DoWorkEv+172)
09-07 15:39:37.378: A/DEBUG(26099):     #10 pc 00092da7  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base18MessagePumpDefault3RunEPNS_11MessagePump8DelegateE+206)
09-07 15:39:37.378: A/DEBUG(26099):     #11 pc 00090e43  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base11MessageLoop10RunHandlerEv+86)
09-07 15:39:37.378: A/DEBUG(26099):     #12 pc 000a8b5f  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so (_ZN4base7RunLoop3RunEv+30)
09-07 15:39:37.378: A/DEBUG(26099):     #13 pc 00041c73  /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.cr.so
09-07 15:39:37.378: A/DEBUG(26099):     #14 pc 000c4ecf  /data/app/org.chromium.chromoting-2/lib/arm/libbase.cr.so
09-07 15:39:37.378: A/DEBUG(26099):     #15 pc 00047003  /system/lib/libc.so (_ZL15__pthread_startPv+22)
09-07 15:39:37.378: A/DEBUG(26099):     #16 pc 00019e1d  /system/lib/libc.so (__start_thread+6)

```

Translated:


```

  RELADDR   FUNCTION                                                                                                                                                                                                                                                 FILE:LINE
  000475e6  pthread_mutex_lock+1                                                                                                                                                                                                                                     /system/lib/libc.so
  000b6627  base::internal::LockImpl::Lock()                                                                                                                                                                                                                         /usr/local/google/home/yuweih/Workspace/Chromium/src/base/synchronization/lock_impl_posix.cc:64
  v------>  base::Lock::Acquire()                                                                                                                                                                                                                                    /usr/local/google/home/yuweih/Workspace/Chromium/src/base/synchronization/lock.h:45
  v------>  AutoLock                                                                                                                                                                                                                                                 /usr/local/google/home/yuweih/Workspace/Chromium/src/base/synchronization/lock.h:115
  000c98c3  base::ThreadCheckerImpl::CalledOnValidThread() const                                                                                                                                                                                                     /usr/local/google/home/yuweih/Workspace/Chromium/src/base/threading/thread_checker_impl.cc:19
  000499c7  remoting::GlCanvas::SetTransformationMatrix(std::__1::array<float, 9u> const&)                                                                                                                                                                           /usr/local/google/home/yuweih/Workspace/Chromium/src/remoting/client/gl_canvas.cc:100
  0004b627  remoting::GlRenderer::OnPixelTransformationChanged(std::__1::array<float, 9u> const&)                                                                                                                                                                    /usr/local/google/home/yuweih/Workspace/Chromium/src/remoting/client/gl_renderer.cc:47
  v------>  base::Callback<void (), (base::internal::CopyMode)1>::Run() const                                                                                                                                                                                        /usr/local/google/home/yuweih/Workspace/Chromium/src/base/callback.h:61
  00233259  remoting::ConsumeTaskQueue(std::__1::queue<base::Callback<void (), (base::internal::CopyMode)1>, std::__1::deque<base::Callback<void (), (base::internal::CopyMode)1>, std::__1::allocator<base::Callback<void (), (base::internal::CopyMode)1> > > >*)  /usr/local/google/home/yuweih/Workspace/Chromium/src/remoting/client/queued_task_poster.cc:37
  v------>  base::Callback<void (), (base::internal::CopyMode)1>::Run() const                                                                                                                                                                                        /usr/local/google/home/yuweih/Workspace/Chromium/src/base/callback.h:61
  0007a045  base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask const&)                                                                                                                                                                               /usr/local/google/home/yuweih/Workspace/Chromium/src/base/debug/task_annotator.cc:54
  00091045  base::MessageLoop::RunTask(base::PendingTask const&)                                                                                                                                                                                                     /usr/local/google/home/yuweih/Workspace/Chromium/src/base/message_loop/message_loop.cc:488
  000911e7  base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)                                                                                                                                                                                              /usr/local/google/home/yuweih/Workspace/Chromium/src/base/message_loop/message_loop.cc:497
  00091365  base::MessageLoop::DoWork()                                                                                                                                                                                                                              /usr/local/google/home/yuweih/Workspace/Chromium/src/base/message_loop/message_loop.cc:621
  00092da7  base::MessagePumpDefault::Run(base::MessagePump::Delegate*)                                                                                                                                                                                              /usr/local/google/home/yuweih/Workspace/Chromium/src/base/message_loop/message_pump_default.cc:35
  00090e43  base::MessageLoop::RunHandler()                                                                                                                                                                                                                          /usr/local/google/home/yuweih/Workspace/Chromium/src/base/message_loop/message_loop.cc:451
  000a8b5f  base::RunLoop::Run()                                                                                                                                                                                                                                     /usr/local/google/home/yuweih/Workspace/Chromium/src/base/run_loop.cc:35
  00041c73  remoting::AutoThread::ThreadMain()                                                                                                                                                                                                                       /usr/local/google/home/yuweih/Workspace/Chromium/src/remoting/base/auto_thread.cc:211
  000c4ecf  base::(anonymous namespace)::ThreadFunc(void*)                                                                                                                                                                                                           /usr/local/google/home/yuweih/Workspace/Chromium/src/base/threading/platform_thread_posix.cc:71
  00047003  __pthread_start(void*)+22                                                                                                                                                                                                                                /system/lib/libc.so
  00019e1d  __start_thread+6                                                                                                                                                                                                                                         /system/lib/libc.so

```

What really happens is GlRenderer::SurfaceCreated has not been called before calling GlRenderer::OnPixelTransformationChanged resulting in calling SetTransformationMatrix on the nullptr |canvas_|. And there are two possible causes:

1. The transformation has been set before SurfaceCreated.
2. The surface is created before the surface callback is registered. Causing GlRenderer::SurfaceCreated not being called at all.

If #1 happens then simply adding the check should fix the problem. #2 is not likely to happen since the callback is registered directly inside the constructor. But if that really happens, then we will need to investigate more on this issue.
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 9 2016 

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

commit ac2e73718fbdfbb5cb5458a1271339174deec8ef
Author: yuweih <yuweih@chromium.org>
Date: Fri Sep 09 00:51:11 2016

[Remoting Android] Fix OnPixelTransformationChanged Flakiness

In very rare circumstance (<5% connection attempt)
GlRenderer::OnPixelTransformationChanged is called when SurfaceCreated has not
been called, which crashes the app since |canvas_| has not been set. This CL
adds check for |canvas_| to prevent crashing. For now we don't know in that
case whether the transformation was set before SurfaceCreated or the surface
had actually been created before the surface callback was registered. If former
is true then this CL will fix the problem since SurfaceCreated will implicitly
cause TransformationChanged. If latter is true then nothing will be drawn on
the view although the app doesn't crash, and we will need to investigate why
this can ever happen.

BUG= 644924 

Review-Url: https://codereview.chromium.org/2322623003
Cr-Commit-Position: refs/heads/master@{#417463}

[modify] https://crrev.com/ac2e73718fbdfbb5cb5458a1271339174deec8ef/remoting/client/gl_renderer.cc

Comment 2 by yuweih@chromium.org, Sep 9 2016

Status: Fixed (was: Assigned)
Not sure what is the best way to repro this other than rotating the screen for hundreds of times and see whether the pthread_mutex_lock crash shows up...

If the crash is noticeable on M53 then we may also want to merge this to M53.

Comment 3 by yuweih@chromium.org, Sep 9 2016 

I mean M54, not M53...

The bug was introduced by the OpenGL renderer so it didn't exist in earlier build.

Comment 4 by ajnolley@chromium.org, Sep 16 2016

Status: Verified (was: Fixed)
I was able to repro this once after rotating the screen a number of times, and not at all with a later client build. Verification is a bit iffy since the crash is rare, but I think we can mark this verified.
►

Sign in to add a comment