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

Issue 822798 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

AndroidOverlay leaks mojo router java objects

Project Member Reported by liber...@chromium.org, Mar 16 2018

Issue description

One sees warnings of the form:

02-26 13:42:19.095 E/System  ( 6924): Uncaught exception thrown by finalizer
02-26 13:42:19.096 E/System  ( 6924): java.lang.IllegalStateException: Warning: Router objects should be explicitly closed when no longer required otherwise you may leak handles.
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.AutoCloseableRouter.finalize(AutoCloseableRouter.java:117)
02-26 13:42:19.096 E/System  ( 6924):   at java.lang.Daemons$FinalizerDaemon.doFinalize(Daemons.java:222)
02-26 13:42:19.096 E/System  ( 6924):   at java.lang.Daemons$FinalizerDaemon.run(Daemons.java:209)
02-26 13:42:19.096 E/System  ( 6924):   at java.lang.Thread.run(Thread.java:761)
02-26 13:42:19.096 E/System  ( 6924): Caused by: java.lang.Exception: AutocloseableRouter allocated at:
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.AutoCloseableRouter.<init>(AutoCloseableRouter.java:43)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Interface$Manager.attachProxy(Interface.java:400)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Interface$Manager.attachProxy(Interface.java:364)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Decoder.readServiceInterface(Decoder.java:502)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.media.mojom.AndroidOverlayProvider_Internal$AndroidOverlayProviderCreateOverlayParams.decode(AndroidOverlayProvider_Internal.java:208)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.media.mojom.AndroidOverlayProvider_Internal$AndroidOverlayProviderCreateOverlayParams.deserialize(AndroidOverlayProvider_Internal.java:176)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.media.mojom.AndroidOverlayProvider_Internal$Stub.accept(AndroidOverlayProvider_Internal.java:112)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.RouterImpl.handleIncomingMessage(RouterImpl.java:238)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.RouterImpl.access$000(RouterImpl.java:21)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.RouterImpl$HandleIncomingMessageThunk.accept(RouterImpl.java:33)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Connector.readAndDispatchMessage(Connector.java:204)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Connector.readOutstandingMessages(Connector.java:172)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Connector.onWatcherResult(Connector.java:152)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Connector.access$100(Connector.java:25)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.bindings.Connector$WatcherCallback.onResult(Connector.java:142)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.mojo.system.impl.WatcherImpl.onHandleReady(WatcherImpl.java:53)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce(Native Method)
02-26 13:42:19.096 E/System  ( 6924):   at org.chromium.base.SystemMessageHandler.handleMessage(SystemMessageHandler.java:51)
02-26 13:42:19.096 E/System  ( 6924):   at android.os.Handler.dispatchMessage(Handler.java:102)
02-26 13:42:19.096 E/System  ( 6924):   at android.os.Looper.loop(Looper.java:154)
02-26 13:42:19.096 E/System  ( 6924):   at android.app.ActivityThread.main(ActivityThread.java:6077)
02-26 13:42:19.096 E/System  ( 6924):   at java.lang.reflect.Method.invoke(Native Method)
02-26 13:42:19.096 E/System  ( 6924):   at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
02-26 13:42:19.096 E/System  ( 6924):   at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)

 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 16 2018

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

commit d1414246cfe6782f2906c186021526fbf69cee75
Author: liberato@chromium.org <liberato@chromium.org>
Date: Fri Mar 16 19:00:23 2018

Close AndroidOverlay client to prevent leaked mojo routers.

Previously, DialogOverlayImpl did not close the overlay client when
cleaning up.  This caused the mojo router in |mClient| to be closed
only when the client was GC'd, and issued warnings in logcat.

Now, DialogOverlayImpl closes |mClient| properly.

AndroidOverlayProviderImpl also closes the client if it fails before
providing the client to a DialogOverlayImpl.

Bug:  822798 
Change-Id: I57657b4e1bc77e07e79a1182c5467e243d947002
Reviewed-on: https://chromium-review.googlesource.com/966868
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543770}
[modify] https://crrev.com/d1414246cfe6782f2906c186021526fbf69cee75/content/public/android/java/src/org/chromium/content/browser/androidoverlay/AndroidOverlayProviderImpl.java
[modify] https://crrev.com/d1414246cfe6782f2906c186021526fbf69cee75/content/public/android/java/src/org/chromium/content/browser/androidoverlay/DialogOverlayImpl.java

Needs merging?
Labels: Merge-Request-66
it's mostly log spam, but can't hurt to remove it.
Status: Fixed (was: Started)
Project Member

Comment 5 by sheriffbot@chromium.org, Mar 16 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by cmasso@google.com, Mar 19 2018

Please verify the fix in the latest canary
i can verify that nothing gets worse.  it wasn't causing any crashes or other behavior that would improve with the fix, though.

Comment 8 by cmasso@google.com, Mar 21 2018

Labels: -Hotlist-Merge-Review -Merge-Review-66 Merge-Approved-66
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 21 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6962ed2e167ef7a71b658c4389e23f8873477f6b

commit 6962ed2e167ef7a71b658c4389e23f8873477f6b
Author: liberato@chromium.org <liberato@chromium.org>
Date: Wed Mar 21 22:19:45 2018

Close AndroidOverlay client to prevent leaked mojo routers.

Previously, DialogOverlayImpl did not close the overlay client when
cleaning up.  This caused the mojo router in |mClient| to be closed
only when the client was GC'd, and issued warnings in logcat.

Now, DialogOverlayImpl closes |mClient| properly.

AndroidOverlayProviderImpl also closes the client if it fails before
providing the client to a DialogOverlayImpl.

Bug:  822798 
Change-Id: I57657b4e1bc77e07e79a1182c5467e243d947002
Reviewed-on: https://chromium-review.googlesource.com/966868
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543770}(cherry picked from commit d1414246cfe6782f2906c186021526fbf69cee75)
Reviewed-on: https://chromium-review.googlesource.com/974088
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#370}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/6962ed2e167ef7a71b658c4389e23f8873477f6b/content/public/android/java/src/org/chromium/content/browser/androidoverlay/AndroidOverlayProviderImpl.java
[modify] https://crrev.com/6962ed2e167ef7a71b658c4389e23f8873477f6b/content/public/android/java/src/org/chromium/content/browser/androidoverlay/DialogOverlayImpl.java

Sign in to add a comment