New issue
Advanced search Search tips

Issue 871604 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

StrictMode violation in WebView

Project Member Reported by ntfschr@chromium.org, Aug 7

Issue description

Originally observed in issue 870481 and issue 863361.

Steps to repro:

1. compile and install system_webview_shell_apk
2. adb shell am start \
    -n "org.chromium.webview_shell/.WebViewBrowserActivity" \
    -d "https://edition.cnn.com/2018/07/12/world/neutrino-blazar-cosmic-ray-discovery/index.html"
3. Click play
4. Observe crash (this doesn't repro 100%, so you might need to try a few times).

^ I get a fairly consistent repro with 69.0.3497.24 and above. I can't repro on M68 right now, although I thought I did so previously.

---

The strictmode exception looks like:

E StrictMode: A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
E StrictMode: java.lang.Throwable: Explicit termination method 'release' not called
E StrictMode:    at dalvik.system.CloseGuard.open(CloseGuard.java:180)
E StrictMode:    at android.view.Surface.setNativeObjectLocked(Surface.java:511)
E StrictMode:    at android.view.Surface.<init>(Surface.java:179)
W System.err: StrictMode VmPolicy violation with POLICY_DEATH; shutting down.
I Process : Sending signal. PID: 32311 SIG: 9
W AudioFlinger::EffectModule: EffectModule 0xe776a780 destructor called with unreleased interface
D GraphicsStats: Buffer count: 5
W AudioFlinger::EffectModule: EffectModule 0xe776bc80 destructor called with unreleased interface
I WindowManager: WIN DEATH: Window{661f734 u0 org.chromium.webview_shell/org.chromium.webview_shell.WebViewBrowserActivity}
D ConnectivityService: ConnectivityService NetworkRequestInfo binderDied(NetworkRequest [ LISTEN id=1259, [ Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&FOREGROUND] ], android.os.BinderProxy@90ab9ef)
D ConnectivityService: ConnectivityService NetworkRequestInfo binderDied(NetworkRequest [ LISTEN id=1258, [ Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&FOREGROUND] ], android.os.BinderProxy@c122cfc)
I ActivityManager: Process org.chromium.webview_shell (pid 32311) has died
D ActivityManager: cleanUpApplicationRecord -- 32311
E ConnectivityService: RemoteException caught trying to send a callback msg for NetworkRequest [ LISTEN id=1259, [ Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&FOREGROUND] ]

---

Paul introduced strictmode in the WebView shell (56a6262f1826f71e8426642a498bc0bca92cebbb), so you can only reproduce this with a recent version of the shell.
 
I've attached a recent build of the WebView shell (no modifications, I'm synced at de31a6341849baa4ea04b8e41a2227066fee5f04).
recent-build-webview-shell.apk
708 KB Download
Looks like this is due to .detectLeakedClosableObjects() which was added in Paul's path (as mentioned above https://chromium-review.googlesource.com/c/1103395/).

Ideally a resource should be explicitly closed, only it doesn't seem to be clear from the logs which resource is the reason for the policy violation.

I can reproduce this on 70.0.3517.0 from official builds, but I can't repro on a local build (I have 2 checkouts, synced to 3509 and 3518 respectively, neither of which can repro).
Owner: boliu@chromium.org
Status: Assigned (was: Untriaged)
This line doesn't do what it appears to do because the move operation of ScopedJavaSurface has been buggy since its inception.
https://cs.chromium.org/chromium/src/media/gpu/android/avda_surface_bundle.cc?rcl=1ba1336ef64baafac9a9bc30002b403fdd57ddb6&l=33

Easy fix though: https://chromium-review.googlesource.com/c/chromium/src/+/1178302
Cc: liber...@chromium.org
Oh wow, +liberato fyi.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 21

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

commit 54796409ba38e7f2b4d794dc59b01fb16f6e7448
Author: Bo Liu <boliu@chromium.org>
Date: Tue Aug 21 03:13:54 2018

Release ScopedJavaSurface on overwrite

Also ensure members are initialized so the move constructor doesn't use
uninitialized values.

Bug:  871604 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I9ab800fec100ccf7ec4de3dd63dfcd28b1e5c5df
Reviewed-on: https://chromium-review.googlesource.com/1178302
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584623}
[modify] https://crrev.com/54796409ba38e7f2b4d794dc59b01fb16f6e7448/ui/gl/android/scoped_java_surface.cc
[modify] https://crrev.com/54796409ba38e7f2b4d794dc59b01fb16f6e7448/ui/gl/android/scoped_java_surface.h

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
What were the consequences of leaking these objects? Is this a fix we should merge?
It will be released when released when the java object is finalized:
https://android.googlesource.com/platform/frameworks/base/+/e5ad598d1143ecc86b4d66f70ca098df068031a9/core/java/android/view/Surface.java#226

So unless the order of destruction is important here, it doesn't really matter that much.
> Is this a fix we should merge?

I saw failures on M69, which is a point in favor of merging. That said, I could only repro on the CNN URLs, and only affects apps which turn on StrictMode--I don't know how popular this is, so it's hard to say what the impact is.

Sign in to add a comment