Crash when connecting from Android to any Host |
|||||||
Issue descriptionVersion: 52.0.2743.10 OS: Android What steps will reproduce the problem? (1)Attempt to connect to any host using the version listed (2)~50% of the time it will crash (3)logcat is included This is a regression
,
May 27 2016
,
May 27 2016
I can't repro this on my Nexus 6P anymore (after an Android update, why...) but I can repro this using AJ's tablet.... This can be reproduced by the ToT build, and the log looks like this: 05-27 14:19:19.683: A/chromium(14287): [0527/141919:FATAL:chromoting_jni_instance.cc(96)] Check failed: !view_. 05-27 14:19:19.683: A/chromium(14287): #00 0xe0e14067 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x00b2a067 05-27 14:19:19.683: A/chromium(14287): #01 0xe0392bf1 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000a8bf1 05-27 14:19:19.683: A/chromium(14287): #02 0xe0392d9d /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000a8d9d 05-27 14:19:19.683: A/chromium(14287): #03 0xe0394c23 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000aac23 05-27 14:19:19.683: A/chromium(14287): #04 0xe0394bf7 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000aabf7 05-27 14:19:19.683: A/chromium(14287): #05 0xe0e02dbd /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x00b18dbd 05-27 14:19:19.683: A/chromium(14287): #06 0xe0392423 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000a8423 05-27 14:19:19.683: A/chromium(14287): #07 0xe0397115 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000ad115 05-27 14:19:19.683: A/chromium(14287): #08 0xe03964f1 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000ac4f1 05-27 14:19:19.683: A/chromium(14287): #09 0xe0396455 /data/app/org.chromium.chromoting-2/lib/arm/libremoting_client_jni.so+0x000ac455 05-27 14:19:19.683: A/chromium(14287): #10 0xe1de29b5 /data/app/org.chromium.chromoting-2/oat/arm/base.odex+0x007b99b5 05-27 14:19:19.683: A/libc(14287): Fatal signal 6 (SIGABRT), code -6 in tid 14287 (mium.chromoting) which is exactly the problem we have in 611893 and should be solved in 614242. The log also suggests that invalidating the weak pointer on the network thread doesn't resolve this problem. I tried the build with CL patch 2007123003 and the problem goes away. The thread model in Android client has long been problematic, and became flaky after I committed the CL 1976853002 which made the connectToHost call asynchronous. There are basically two solutions: 1. Merge 2007403002 and 2007123003 after the latter one is done. 1. Make connectToHost synchronous again, which is bad since it doesn't care whether the auth token is stale or not.
,
May 28 2016
Looks like making connectToHost synchronous doesn't really solve this problem. Symbolized logs has shown weird behavior of the refcounter. JniClient holds a reference to ChromotingJniInstance. The network thread holds a reference to the instance for connecting to the host on network inside the ctor of ChromotingJniInstance, and it frees the instance when the task is done, which doesn't make sense since JniClient is still holding a reference to the instance. Lambrous and I have no idea what really happens, but seems like separating the class by threads does solve this problem. ------ Done a continuous connect-and-cancel experiment with CL patch 1998353002. # connect-and-cancel takes to crash: ToT: 12, 5, 3, 3, 3, 2, 26, 9 With patch 2007123003: >340, >532, >314 No crash observed. Started getting XMPP error instead...
,
May 28 2016
Thanks to wez@'s comment on the design doc. I've finally figured out what was going on :) Refcounted object is initialized to have zero refcount and will be increased to 1 in RefCountedThreadSafe::operator=(). There is a chance that ConnectToHost finishes earlier than operator=(), which drops the refcount to 0 and triggers the d'tor. Moving the PostTask code out of the ctor should be enough to solve this problem.
,
May 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/35c9e62b75248238526386edc44c46794adac095 commit 35c9e62b75248238526386edc44c46794adac095 Author: yuweih <yuweih@chromium.org> Date: Tue May 31 22:50:52 2016 Fix bug of posting task inside constructor Refcounted object is initialized to have zero refcount and will be increased to 1 in RefCountedThreadSafe::operator=(). There is a chance that ConnectToHost finishes earlier than operator=(), which drops the refcount to 0 and triggers the d'tor. Adding sleep(1) after PostTask() shows stable repro of this problem. This CL fixes this problem by pulling the PostTask code out of the ctor. BUG= 615288 Review-Url: https://codereview.chromium.org/2023113002 Cr-Commit-Position: refs/heads/master@{#396961} [modify] https://crrev.com/35c9e62b75248238526386edc44c46794adac095/remoting/client/jni/chromoting_jni_instance.cc [modify] https://crrev.com/35c9e62b75248238526386edc44c46794adac095/remoting/client/jni/chromoting_jni_instance.h [modify] https://crrev.com/35c9e62b75248238526386edc44c46794adac095/remoting/client/jni/chromoting_jni_runtime.cc
,
Jun 1 2016
Tested on official build 53.0.2754.0. These changes will only affect the Chrome Remote Desktop Android app and will not affect the Chrome browser.
,
Jun 1 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6ab1822890f8eebdf8aca40a26b4715518a4767 commit c6ab1822890f8eebdf8aca40a26b4715518a4767 Author: Lambros Lambrou <lambroslambrou@chromium.org> Date: Wed Jun 01 17:12:18 2016 Fix bug of posting task inside constructor Refcounted object is initialized to have zero refcount and will be increased to 1 in RefCountedThreadSafe::operator=(). There is a chance that ConnectToHost finishes earlier than operator=(), which drops the refcount to 0 and triggers the d'tor. Adding sleep(1) after PostTask() shows stable repro of this problem. This CL fixes this problem by pulling the PostTask code out of the ctor. BUG= 615288 Review-Url: https://codereview.chromium.org/2023113002 Cr-Commit-Position: refs/heads/master@{#396961} (cherry picked from commit 35c9e62b75248238526386edc44c46794adac095) Review URL: https://codereview.chromium.org/2029103002 . Cr-Commit-Position: refs/branch-heads/2743@{#165} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/c6ab1822890f8eebdf8aca40a26b4715518a4767/remoting/client/jni/chromoting_jni_instance.cc [modify] https://crrev.com/c6ab1822890f8eebdf8aca40a26b4715518a4767/remoting/client/jni/chromoting_jni_instance.h [modify] https://crrev.com/c6ab1822890f8eebdf8aca40a26b4715518a4767/remoting/client/jni/chromoting_jni_runtime.cc
,
Jun 1 2016
,
Jun 1 2016
52.0.2743.23 or later will have the fix.
,
Jun 1 2016
lambroslambrou@, thanks for merging this for me!
,
Jun 2 2016
I made ~20 connection attempts per host platform with no hiccups. Verified fixed in 52.0.2743.26 |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by ajnolley@chromium.org
, May 27 2016