New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 668692 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

creating and destroying webviews leaks InputConnectionHandlerThread threads

Project Member Reported by tobiasjs@chromium.org, Nov 25 2016

Issue description

run:

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

 
This also happens on 54.0.2840.85
Labels: OS-Android
Cc: amineer@chromium.org
Alex, FYI.
Alex is on vacation. Let me take a look.
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?

That looks like your build directory is 32 bit, while your device is 64 bit.
If you want to observe the effect, you don't need to attach with adb; just

cat /proc/$pid/status | grep Threads 
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?
tobiasjs@, could you answer comment #8?
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?
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/

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).
thread cost.png
48.4 KB View Download
Cc: perezju@chromium.org
Cc: picksi@chromium.org
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.
I'll add this to a proposed OKR list
Merge of https://codereview.chromium.org/2537073002/ approved for M55 branch 2883.
Labels: Merge-Approved-55
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 30 2016

Labels: -merge-approved-55 merge-merged-2883
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

Project Member

Comment 19 by bugdroid1@chromium.org, 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

Do we need to merge this to m56 as well?
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.

Comment 22 by torne@chromium.org, Nov 30 2016

This looks to have caused a crash, see issue 669886, so let's not cherrypick it anywhere else yet :/
Project Member

Comment 23 by bugdroid1@chromium.org, 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

Project Member

Comment 24 by bugdroid1@chromium.org, 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

Cc: yabinh@chromium.org
Uploaded https://codereview.chromium.org/2543893002/ for review.
Labels: Merge-Approved-55
Let's get that on M55 branch and built; please CP to M56 as well if necessary.
Project Member

Comment 27 by bugdroid1@chromium.org, 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

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.

Project Member

Comment 29 by bugdroid1@chromium.org, Dec 1 2016

Labels: -merge-approved-55
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

Components: UI>Input>Text>IME
Labels: Merge-Request-56
Labels: -Merge-Request-56 Merge-Approved-56
Approved for M56 branch 2924 as well.
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.
Project Member

Comment 33 by bugdroid1@chromium.org, Dec 1 2016

Labels: -merge-approved-56 merge-merged-2924
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

Status: Fixed (was: Available)
Issue Doesn't repro on Nexus 6P / NMF26P and Samsung On nxt /MMB29K on latest M55:55.0.2883.77 build

Tahnks

Comment 36 by aluo@chromium.org, 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.
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 
Cc: sgu...@chromium.org
Likely not ime issue then.

Toby, is there original crbug for #37?

Ccing sgurun in case he can help investigate in mtv timezone.
b/32901463 seems like the bug. I can take a look in my timezone.
b/33111675 is the original ANR bug for that app.
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.
Thanks, aelias@ for steps in #41 - we are checking.
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.
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 

Comment 45 by aluo@chromium.org, Dec 2 2016

This issue missed today push build for M56, 56.0.2924.13.  Will verify on tomorrow's build.
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!
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.)
Cc: satyavat...@chromium.org aluo@chromium.org alek...@chromium.org
Status: Assigned (was: Fixed)
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.
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?

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.

Comment 51 by aluo@chromium.org, Dec 3 2016

Today's build failed (https://bugs.chromium.org/p/chromium/issues/detail?id=670922) , will verify on M56 Monday.
Cc: tobiasjs@chromium.org
Owner: tobiasjs@chromium.org
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.

Cc: boliu@chromium.org
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.

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 ;)
Project Member

Comment 56 by bugdroid1@chromium.org, 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

Cc: changwan@chromium.org
Labels: Merge-Request-56
Requesting a merge for ff08fc80 to m56. It should have been tagged with a different bug, but was discovered during the same tunein investigation.

Comment 59 by dimu@chromium.org, Dec 7 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 60 by bugdroid1@chromium.org, Dec 7 2016

Labels: -merge-approved-56
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

 Issue 672369  has been merged into this issue.

Comment 62 by aluo@chromium.org, Dec 12 2016

Verified issue fixed in 56.0.2924.23, tunein.player hasn't crashed after hours of playing.  Device LG V20, NRD90M.
Status: Fixed (was: Assigned)

Sign in to add a comment