creating and destroying webviews leaks InputConnectionHandlerThread threads |
||||||||||||||||||||||
Issue descriptionrun: am start -n org.chromium.webview_shell/org.chromium.webview_shell.WebViewCreateDestroyActivity a handful of times, attach gdb with: ./build/android/adb_gdb --package-name=org.chromium.webview_shell --symbol-dir=webview-55.0.2883.53/Release/lib.unstripped --pull-libs and see, e.g.: (gdb) info threads ... 46 Thread 28180.28377 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 47 Thread 28180.28414 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 48 Thread 28180.28442 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 49 Thread 28180.28455 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 50 Thread 28180.28502 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 51 Thread 28180.28534 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 52 Thread 28180.28561 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 53 Thread 28180.29799 "Binder_3" 0xb686ef0c in __ioctl () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 54 Thread 28180.29894 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 55 Thread 28180.29900 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 56 Thread 28180.29907 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 57 Thread 28180.29908 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 58 Thread 28180.29928 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 59 Thread 28180.29931 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 60 Thread 28180.29932 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 61 Thread 28180.29933 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 62 Thread 28180.29934 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 63 Thread 28180.29936 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 64 Thread 28180.29937 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 65 Thread 28180.29939 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 66 Thread 28180.29943 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 67 Thread 28180.29944 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so 68 Thread 28180.29945 "InputConnection" 0xb686f0e8 in __epoll_pwait () from /tmp/tobiasjs-adb-gdb-libs/system/lib/libc.so There's only one AwContents instance: (gdb) p 'android_webview::(anonymous namespace)::g_instance_count' $1 = 1
,
Nov 25 2016
,
Nov 25 2016
Alex, FYI.
,
Nov 28 2016
Alex is on vacation. Let me take a look.
,
Nov 28 2016
When I try the following command, ./build/android/adb_gdb --package-name=org.chromium.webview_shell --symbol-dir=out_android_gn/Debug/lib.unstripped --pull-libs I get a lot of warnings: phics.so" is not at the expected address (wrong library or version mismatch?) warning: `/tmp/changwan-adb-gdb-libs/system/lib64/libwebviewchromium_loader.so': Shared library architecture aarch64 is not compatible with target architecture arm. warning: .dynamic section for "/tmp/changwan-adb-gdb-libs/system/lib64/libwebviewchromium_loader.so" is not at the expected address (wrong library or version mismatch?) warning: `/tmp/changwan-adb-gdb-libs/system/lib64/libwebviewchromium_plat_support.so': Shared library architecture aarch64 is not compatible with target architecture arm. warning: .dynamic section for "/tmp/changwan-adb-gdb-libs/system/lib64/libwebviewchromium_plat_support.so" is not at the expected address (wrong library or version mismatch?) warning: `/tmp/changwan-adb-gdb-libs/system/lib64/libRSCpuRef.so': Shared library architecture aarch64 is not compatible with target architecture arm. warning: .dynamic section for "/tmp/changwan-adb-gdb-libs/system/lib64/libRSCpuRef.so" is not at the expected address (wrong library or version mismatch?) warning: `/tmp/changwan-adb-gdb-libs/system/lib64/libRSDriver.so': Shared library architecture aarch64 is not compatible with target architecture arm. warning: .dynamic section for "/tmp/changwan-adb-gdb-libs/system/lib64/libRSDriver.so" is not at the expected address (wrong library or version mismatch?) warning: Could not load shared library symbols for libwebviewchromium.so. Do you need "set solib-search-path" or "set sysroot"? Is there any detailed instruction on how to run the test commands?
,
Nov 28 2016
That looks like your build directory is 32 bit, while your device is 64 bit.
,
Nov 28 2016
If you want to observe the effect, you don't need to attach with adb; just cat /proc/$pid/status | grep Threads
,
Nov 29 2016
If this is going to block M55 it needs to be addressed yesterday. We might still be able to take a fix but it's coming way late. What's the impact of this regression? Can it be quantified? E.g. if this is causing memory leaks, why haven't our perf infra bots caught it?
,
Nov 29 2016
tobiasjs@, could you answer comment #8?
,
Nov 29 2016
Re #7, I get Threads: 37 Probably something went wrong... tobiasjs@, could you let me know the exact build command and the device that you're using?
,
Nov 29 2016
Ok, re #10, the point is to make sure the number does not grow. I've uploaded a patch to address this issue: https://codereview.chromium.org/2537073002/
,
Nov 29 2016
We believe that the thread leak is responsible for TuneIn radio ANRs. It creates and destroys WebViews at ~1 per 5 seconds to display ads. It's not clear whether this create-destroy cycle is common for ad SDKs, but if it is, then many apps could be affected. Looking at a process on a Nexus 5X, it looks like android reserves 1M of address space per thread. The actual PSS cost is about 48 KB per thread (see graph).
,
Nov 29 2016
,
Nov 29 2016
This kind of issues should be caught by a benchmark like the one described in issue 619812 . Simon, we should probably put that benchmark on our OKRs for next quarter.
,
Nov 29 2016
I'll add this to a proposed OKR list
,
Nov 30 2016
Merge of https://codereview.chromium.org/2537073002/ approved for M55 branch 2883.
,
Nov 30 2016
,
Nov 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7860ebf0f5005dfe519f5abc0a352ce680144f5 commit c7860ebf0f5005dfe519f5abc0a352ce680144f5 Author: Changwan Ryu <changwan@google.com> Date: Wed Nov 30 01:45:37 2016 Fix leaks in InputConnectionHandlerThread On destruction of ContentViewCore and on view detachment, we reset mHandler, allowing it a chance to be garbage-collected. BUG= 668692 (directly cherry-picked from patchset http://crrev.com/2537073002#ps20001 for early QA pass purposes, as requested by TPM) Review URL: https://codereview.chromium.org/2532383003 . Cr-Commit-Position: refs/branch-heads/2883@{#689} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/c7860ebf0f5005dfe519f5abc0a352ce680144f5/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java [modify] https://crrev.com/c7860ebf0f5005dfe519f5abc0a352ce680144f5/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java [modify] https://crrev.com/c7860ebf0f5005dfe519f5abc0a352ce680144f5/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java [modify] https://crrev.com/c7860ebf0f5005dfe519f5abc0a352ce680144f5/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java [modify] https://crrev.com/c7860ebf0f5005dfe519f5abc0a352ce680144f5/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java [modify] https://crrev.com/c7860ebf0f5005dfe519f5abc0a352ce680144f5/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/c7860ebf0f5005dfe519f5abc0a352ce680144f5/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java
,
Nov 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b commit c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b Author: changwan <changwan@chromium.org> Date: Wed Nov 30 07:44:34 2016 Fix leaks in InputConnectionHandlerThread On destruction of ContentViewCore and on view detachment, we reset mHandler, allowing it a chance to be garbage-collected. BUG= 668692 Review-Url: https://codereview.chromium.org/2537073002 Cr-Commit-Position: refs/heads/master@{#435136} [modify] https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java [modify] https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java [modify] https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java [modify] https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java [modify] https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java [modify] https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java
,
Nov 30 2016
Do we need to merge this to m56 as well?
,
Nov 30 2016
As this has been cherry picked into m55 we should definitely also get this into m56. If we're lucky the fix in ToT might help our reported System Health numbers, as we are still far from green.
,
Nov 30 2016
This looks to have caused a crash, see issue 669886, so let's not cherrypick it anywhere else yet :/
,
Nov 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94e8585dbf62dd31236951004ce9ac785c6409b6 commit 94e8585dbf62dd31236951004ce9ac785c6409b6 Author: Alex Mineer <amineer@chromium.org> Date: Wed Nov 30 18:42:36 2016 Revert "Fix leaks in InputConnectionHandlerThread" This reverts commit c7860ebf0f5005dfe519f5abc0a352ce680144f5. Reverting on M55 branch 2883 due to crash bug. BUG=669886, 668692 Cr-Commit-Position: refs/branch-heads/2883@{#696} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/94e8585dbf62dd31236951004ce9ac785c6409b6/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java [modify] https://crrev.com/94e8585dbf62dd31236951004ce9ac785c6409b6/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java [modify] https://crrev.com/94e8585dbf62dd31236951004ce9ac785c6409b6/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java [modify] https://crrev.com/94e8585dbf62dd31236951004ce9ac785c6409b6/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java [modify] https://crrev.com/94e8585dbf62dd31236951004ce9ac785c6409b6/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java [modify] https://crrev.com/94e8585dbf62dd31236951004ce9ac785c6409b6/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/94e8585dbf62dd31236951004ce9ac785c6409b6/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0b968c4c01cc7c20f7f8c807a566def4559965af commit 0b968c4c01cc7c20f7f8c807a566def4559965af Author: changwan <changwan@chromium.org> Date: Thu Dec 01 00:55:18 2016 Revert of Fix leaks in InputConnectionHandlerThread (patchset #2 id:20001 of https://codereview.chromium.org/2537073002/ ) Reason for revert: caused crbug.com/669886 Original issue's description: > Fix leaks in InputConnectionHandlerThread > > On destruction of ContentViewCore and on view detachment, we reset mHandler, > allowing it a chance to be garbage-collected. > > BUG= 668692 > > Committed: https://crrev.com/c703b49a25b8b279fe7cbd1fbfe7ff67493fbf7b > Cr-Commit-Position: refs/heads/master@{#435136} TBR=tobiasjs@chromium.org,aelias@chromium.org,yabinh@chromium.org,torne@chromium.org,tedchoc@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 668692 Review-Url: https://codereview.chromium.org/2546613002 Cr-Commit-Position: refs/heads/master@{#435506} [modify] https://crrev.com/0b968c4c01cc7c20f7f8c807a566def4559965af/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java [modify] https://crrev.com/0b968c4c01cc7c20f7f8c807a566def4559965af/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java [modify] https://crrev.com/0b968c4c01cc7c20f7f8c807a566def4559965af/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java [modify] https://crrev.com/0b968c4c01cc7c20f7f8c807a566def4559965af/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java [modify] https://crrev.com/0b968c4c01cc7c20f7f8c807a566def4559965af/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java [modify] https://crrev.com/0b968c4c01cc7c20f7f8c807a566def4559965af/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/0b968c4c01cc7c20f7f8c807a566def4559965af/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java
,
Dec 1 2016
,
Dec 1 2016
Let's get that on M55 branch and built; please CP to M56 as well if necessary.
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43fae8979739f1ef5620996a9ac978e100c0ec7c commit 43fae8979739f1ef5620996a9ac978e100c0ec7c Author: changwan <changwan@chromium.org> Date: Thu Dec 01 05:07:19 2016 Reuse InputConnectionHandlerThread This can prevent the number of threads from growing when an app creates and destroys webviews. BUG= 668692 Review-Url: https://codereview.chromium.org/2543893002 Cr-Commit-Position: refs/heads/master@{#435557} [modify] https://crrev.com/43fae8979739f1ef5620996a9ac978e100c0ec7c/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
,
Dec 1 2016
My manual verification steps:
1) Build and deploy webview shell:
buildtools/linux64/gn gen out_android_gn/Debug --args='target_os="android" is_component_build=true symbol_level=1 use_goma=true is_java_debug=true enable_incremental_javac=true google_play_services_type="standard" disable_incremental_isolated_processes=true use_signing_keys=true is_debug=false use_webview_internal_framework=true use_unpublished_apis=true'
ninja -C out_android_gn/Debug -j3000 -l15 system_webview_shell_apk && \
adb install -r out_android_gn/Debug/apks/SystemWebViewShell.apk
2) Launch webviewcreatedestroyactivity
adb shell am start -n org.chromium.webview_shell/org.chromium.webview_shell.WebViewCreateDestroyActivity
3) Check the number of threads for webview shell process
adb shell cat /proc/`adb shell ps | grep webview_shell | awk '{print $2}'`/status | grep Threads
Repeat 2) and 3) and see if the number grows.
I haven't checked yet, but according to #12, we may need to check if the webview advertisements in TuneIn Radio do not cause ANR after some hours.
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e2a7a54cbbcbb570781ccf9bbb6052a9b8e9b79 commit 2e2a7a54cbbcbb570781ccf9bbb6052a9b8e9b79 Author: Changwan Ryu <changwan@google.com> Date: Thu Dec 01 05:18:49 2016 Reuse InputConnectionHandlerThread This can prevent the number of threads from growing when an app creates and destroys webviews. BUG= 668692 Review-Url: https://codereview.chromium.org/2543893002 Cr-Commit-Position: refs/heads/master@{#435557} (cherry picked from commit 43fae8979739f1ef5620996a9ac978e100c0ec7c) Review URL: https://codereview.chromium.org/2544753002 . Cr-Commit-Position: refs/branch-heads/2883@{#701} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/2e2a7a54cbbcbb570781ccf9bbb6052a9b8e9b79/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
,
Dec 1 2016
,
Dec 1 2016
Approved for M56 branch 2924 as well.
,
Dec 1 2016
re #28: Thank you for the steps -changwan@. Not every TE has build and deploy access except aluo@- he will verify tomorrow. For now, when new build comes- India team can verify with the webview advertisements in TuneIn Radio.
,
Dec 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/88da98abae4978193424d55c94dba010e6bd0833 commit 88da98abae4978193424d55c94dba010e6bd0833 Author: Changwan Ryu <changwan@google.com> Date: Thu Dec 01 06:58:53 2016 Reuse InputConnectionHandlerThread This can prevent the number of threads from growing when an app creates and destroys webviews. BUG= 668692 Review-Url: https://codereview.chromium.org/2543893002 Cr-Commit-Position: refs/heads/master@{#435557} (cherry picked from commit 43fae8979739f1ef5620996a9ac978e100c0ec7c) Review URL: https://codereview.chromium.org/2544513003 . Cr-Commit-Position: refs/branch-heads/2924@{#239} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/88da98abae4978193424d55c94dba010e6bd0833/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
,
Dec 1 2016
,
Dec 1 2016
Issue Doesn't repro on Nexus 6P / NMF26P and Samsung On nxt /MMB29K on latest M55:55.0.2883.77 build Tahnks
,
Dec 1 2016
Verified on LG V20 NRD90M using 55.0.2883.77, I no longer see the growing list of input connection threads in webview shell.
,
Dec 1 2016
I'm able to get ANR. logcat -> http://go/chrome-androidlogs1/6/668692 I just let it play in the foreground and after 15-20 mins of playing and displaying ads, app becomes non-responsive on nexus 6p NMF26F vs 55.0.2883.77
,
Dec 1 2016
Likely not ime issue then. Toby, is there original crbug for #37? Ccing sgurun in case he can help investigate in mtv timezone.
,
Dec 1 2016
b/32901463 seems like the bug. I can take a look in my timezone.
,
Dec 1 2016
b/33111675 is the original ANR bug for that app.
,
Dec 1 2016
Test-team, to stress test the IME initialization code, please do the following in each major email app (GMail, Inbox, Samsung Email, LG Email, HTC Email): 1) From the message overview, tap "Reply" on an email. 2) Tap on the "Body" area and type "Hello." 3) Delete the draft and return to the message overview. Repeat steps 1-3 about 10 times each.
,
Dec 1 2016
Thanks, aelias@ for steps in #41 - we are checking.
,
Dec 1 2016
In the logcat for #37, there is only one InputConnectionHandlerThread, so the leak is definitely fixed. The logcat says the following, indicating it's a different IME issue: 12-01 10:59:49.810 917 948 E ActivityManager: ANR in tunein.player (tunein.player/tunein.ui.actvities.PlayerActivity) 12-01 10:59:49.810 917 948 E ActivityManager: PID: 28002 12-01 10:59:49.810 917 948 E ActivityManager: Reason: Input dispatching timed out (Waiting to send key event because the focused window has not finished processing all of the input events that were previously delivered to it. Outbound queue length: 0. Wait queue length: 1.) (Note this is different ANR pattern from b/32901463). It seems TuneIn is programmatically sending fake key events, not just simply creating the WebView (not obvious why ads ought to be getting a key event, that's probably some manner of workaround for something). That seems to be tickling multiple IME bugs. We might recommend to them as a workaround that they stop sending these key events.
,
Dec 1 2016
WebView team have completed testing on different OEM (Samsung, LG, HTC, Acer) emails, Gmail and Inbox by Gmail Devices: Samsung Galaxy S7/M, LG G4/M, Nexus 6/N, HTC One A9/M, Acer Predator/L, Samsung Galaxy S6 edge T-MOBILE/L We didn't see any issues there
,
Dec 2 2016
This issue missed today push build for M56, 56.0.2924.13. Will verify on tomorrow's build.
,
Dec 2 2016
Hi all, Glad to hear the leak is fixed! So if I understand this correctly, TuneIn will still see the issue after the build is released? Are the ANRs still happening at the same rate? Should we open a separate bug for the key events or is this something solely up to them to fix? Are there any details on what the issue is and how they need to fix it that I can communicate to them? Thanks!
,
Dec 2 2016
Can test team test the ANR case against M53? That can probably help isolate whether this is IME issue or not. So far I couldn't find a way to find station with ads. (Is it geo blocked? I can only subscribe to 1 year subscription with free 30 days.)
,
Dec 2 2016
Re #46, I think yes and yes. Not sure if it's still an IME issue or not. If it's an entirely different issue, I prefer to keep a separate crbug. Let me first reopen this.
,
Dec 2 2016
I've been playing this app for more than 5 hours but still no luck in reproducing the ANR. BTW, I don't observe any View#dispatchKeyEvent() coming to ImeAdapter. Could ActivityManager log simply implies that 'back' button after ANR happens was not handled? tobiasjs@, in b/33111675, you commented that "fixing [the thread leak] appears to stop the ANR" Is there anything else you suspect as the cause of this ANR?
,
Dec 2 2016
No, we didn't see any underlying cause. Resolving the leak was a speculative fix that locally fixed the issue. I would guess from things that aelias has said that there are other IME related issues that this app is triggering, and the thread leak exacerbates them. (In gb) almost all streams have ads. You should not need to sign up for anything. I'll try again to repro post the thread fix.
,
Dec 3 2016
Today's build failed (https://bugs.chromium.org/p/chromium/issues/detail?id=670922) , will verify on M56 Monday.
,
Dec 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4df6fe2be559a7b87411d369c401efd62ed01353 commit 4df6fe2be559a7b87411d369c401efd62ed01353 Author: changwan <changwan@chromium.org> Date: Mon Dec 05 02:02:03 2016 Simplify handler initialization for InputConnectionFactory As a follow-up to https://codereview.chromium.org/2543893002/, we make the code easy to read and remove unnecessary findbug suppression. BUG= 668692 Review-Url: https://codereview.chromium.org/2544163003 Cr-Commit-Position: refs/heads/master@{#436218} [modify] https://crrev.com/4df6fe2be559a7b87411d369c401efd62ed01353/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java [modify] https://crrev.com/4df6fe2be559a7b87411d369c401efd62ed01353/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java
,
Dec 5 2016
I ran the app over the weekend with added IME logging, and this morning I found the app was non-responsive. Here's my observation: 1) I got "ANR in tunein.player" log after I try to interact with the app, not before it. 12-05 10:34:04.756 947 1202 E ActivityManager: ANR in tunein.player (tunein.player/tunein.activities.ProfileActivity) 12-05 10:34:04.756 947 1202 E ActivityManager: PID: 6309 12-05 10:34:04.756 947 1202 E ActivityManager: Reason: Input dispatching timed out (Waiting to send key event because the focused window has not finished processing all of the input events that were previously delivered to it. Outbound queue length: 0. Wait queue length: 9.) It occurred when I pressed 'back' button when ANR is observed. This also occurs when you touch a button (you get slightly different log message, though.): ANR in tunein.player (tunein.player/tunein.activities.ProfileActivity) 12-05 10:15:18.580 947 1202 E ActivityManager: PID: 6309 12-05 10:15:18.580 947 1202 E ActivityManager: Reason: Input dispatching timed out (Waiting to send non-key event because the touched window has not finished processing certain input events that were delivered to it over 500.0ms ago. Wait queue length: 9. Wait queue head age: 14817.9ms.) 2) No fake key was dispatched to ImeAdapter. I turned on DEBUG logging for Java-side IME related classes, but there was no cr_Ime log saying 'dispatchKeyEvent'. This means that there was no key press at all. 3) IME thread was not paused. I checked eclipse + DDMS and paused the IME thread to see what's going on, but it was looping on a message queue. There is currently no reason to suspect that this is caused by IME changes. I couldn't figure out what caused ANR, either. When I paused main thread, I saw that thread was running a loop from AwGLFunctor, but I guess it's not new information. Assigning to tobiasjs@ in case we want to reuse this crbug.
,
Dec 5 2016
Hmm... I did a bit more investigation. There are several threads that are marked 'blocked' - from #37 and from my own traces.txt. 1) HeapTaskDaemon - this is Android's own daemon. There are several of them that are marked as 'blocked'. 2) CleanupReference - it's running an infinite loop waiting for cleanup to finish. Could the ANR caused by memory leak? If so, can we log why this is happening? Also, is there any chance that there's a deadlock somewhere? cc'ing boliu@ as he is the author of CleanupReference.
,
Dec 5 2016
It's normal for HeapTaskDaemon to be blocked there: it's waiting on a condition variable associated with its task processor. There's no tasks for it to run so it's waiting to be told that there are. The CleanupReference thread is just waiting for the main thread to release the lock protecting CleanupReference tasks and is not participating in a deadlock. The problem is that the main thread is trying to acquire a native lock - Toby says he just figured this out and the lock it's waiting for is actually a lock it's already holding (base::Lock is not reentrant) and so the main thread is deadlocked with respect to *itself*. I'll leave him to explain further ;)
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff08fc807243b2317ef1c8b42f3f423496d12a76 commit ff08fc807243b2317ef1c8b42f3f423496d12a76 Author: tobiasjs <tobiasjs@chromium.org> Date: Tue Dec 06 19:05:01 2016 aw: Fix race between first frame draw and view detach causing deadlock. If a AwContents.onDetachedFromWindow occurs at simulaneously with RenderThreadManager::DrawGL, then the following sequence of events can occur: * RenderThreadManager::DrawGL creates hardware_renderer_ because a frame is ready to be committed to the HardwareRenderer. * BrowserViewRenderer::OnDetachedFromWindow calls the uncommitted frame to be returned to the child compositor. * RenderThreadManager tries to commit the frame that is just returned, leaving it in a state where neither the RenderThreadManager, nor its HardwareRenderer has a frame. Then subsequently when deleting native objects, DeleteHardwareRendererOnUI does no work, meaning that hardware_renderer_ remains set at the time that the RenderThreadManager destructor is called, leading to a DCHECK, or deadlock. In order to avoid this, we consider RenderThreadManager to have a frame (and thus need to run the synchronous RenderThread cleanup in DeleteHardwareRendererOnUI) if it has ever received a frame (stored in has_received_frame_), rather than if it or hardware_renderer_ currently has a frame. In order that BrowserViewRenderer receives the right signal post deletion of hardware_renderer_, we reset has_received_frame_ when RenderThreadManager deletes hardware_renderer_, if RenderThreadManager does not currently have a frame to commit to a new HardwareRenderer instance. BUG= 668692 Review-Url: https://codereview.chromium.org/2560463002 Cr-Commit-Position: refs/heads/master@{#436672} [modify] https://crrev.com/ff08fc807243b2317ef1c8b42f3f423496d12a76/android_webview/browser/browser_view_renderer.h [modify] https://crrev.com/ff08fc807243b2317ef1c8b42f3f423496d12a76/android_webview/browser/render_thread_manager.cc [modify] https://crrev.com/ff08fc807243b2317ef1c8b42f3f423496d12a76/android_webview/browser/render_thread_manager.h
,
Dec 7 2016
,
Dec 7 2016
Requesting a merge for ff08fc80 to m56. It should have been tagged with a different bug, but was discovered during the same tunein investigation.
,
Dec 7 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45dbc1970abbac1c4a48da2ba2075205f397ae09 commit 45dbc1970abbac1c4a48da2ba2075205f397ae09 Author: Tobias Sargeant <tobiasjs@google.com> Date: Wed Dec 07 11:27:15 2016 aw: Fix race between first frame draw and view detach causing deadlock. If a AwContents.onDetachedFromWindow occurs at simulaneously with RenderThreadManager::DrawGL, then the following sequence of events can occur: * RenderThreadManager::DrawGL creates hardware_renderer_ because a frame is ready to be committed to the HardwareRenderer. * BrowserViewRenderer::OnDetachedFromWindow calls the uncommitted frame to be returned to the child compositor. * RenderThreadManager tries to commit the frame that is just returned, leaving it in a state where neither the RenderThreadManager, nor its HardwareRenderer has a frame. Then subsequently when deleting native objects, DeleteHardwareRendererOnUI does no work, meaning that hardware_renderer_ remains set at the time that the RenderThreadManager destructor is called, leading to a DCHECK, or deadlock. In order to avoid this, we consider RenderThreadManager to have a frame (and thus need to run the synchronous RenderThread cleanup in DeleteHardwareRendererOnUI) if it has ever received a frame (stored in has_received_frame_), rather than if it or hardware_renderer_ currently has a frame. In order that BrowserViewRenderer receives the right signal post deletion of hardware_renderer_, we reset has_received_frame_ when RenderThreadManager deletes hardware_renderer_, if RenderThreadManager does not currently have a frame to commit to a new HardwareRenderer instance. BUG= 668692 Review-Url: https://codereview.chromium.org/2560463002 Cr-Commit-Position: refs/heads/master@{#436672} (cherry picked from commit ff08fc807243b2317ef1c8b42f3f423496d12a76) Review URL: https://codereview.chromium.org/2558763002 . Cr-Commit-Position: refs/branch-heads/2924@{#376} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/45dbc1970abbac1c4a48da2ba2075205f397ae09/android_webview/browser/browser_view_renderer.h [modify] https://crrev.com/45dbc1970abbac1c4a48da2ba2075205f397ae09/android_webview/browser/render_thread_manager.cc [modify] https://crrev.com/45dbc1970abbac1c4a48da2ba2075205f397ae09/android_webview/browser/render_thread_manager.h
,
Dec 9 2016
Issue 672369 has been merged into this issue.
,
Dec 12 2016
Verified issue fixed in 56.0.2924.23, tunein.player hasn't crashed after hours of playing. Device LG V20, NRD90M.
,
Dec 13 2016
|
||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||
Comment 1 by tobiasjs@chromium.org
, Nov 25 2016