New issue
Advanced search Search tips

Issue 834099 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

getLastReportedPressure asserting in child process

Project Member Reported by boliu@chromium.org, Apr 17 2018

Issue description

I got a bunch of these while running locally:
out/Default/bin/run_content_browsertests --num_retries=0 -v -f IndexedDBBrowserTest.ConnectionsClosedOnTabClose

04-17 23:51:33.240 10325 10337 E JavaBinder: *** Uncaught remote exception!  (Exceptions are not yet supported across processes.)
04-17 23:51:33.240 10325 10337 E JavaBinder: java.lang.AssertionError: Must be called on the UI thread.
04-17 23:51:33.240 10325 10337 E JavaBinder:    at org.chromium.base.ThreadUtils.assertOnUiThread(ThreadUtils.java:203)
04-17 23:51:33.240 10325 10337 E JavaBinder:    at org.chromium.base.memory.MemoryPressureMonitor.getLastReportedPressure(MemoryPressureMonitor.java:194)
04-17 23:51:33.240 10325 10337 E JavaBinder:    at org.chromium.base.process_launcher.ChildProcessService$1.onMemoryPressure(ChildProcessService.java:154)
04-17 23:51:33.240 10325 10337 E JavaBinder:    at org.chromium.base.process_launcher.IChildProcessService$Stub.onTransact(IChildProcessService.java:81)
04-17 23:51:33.240 10325 10337 E JavaBinder:    at android.os.Binder.execTransact(Binder.java:453)
04-17 23:51:33.240 10325 10337 W System.err: java.lang.AssertionError: Must be called on the UI thread.
04-17 23:51:33.240 10325 10337 W System.err:    at org.chromium.base.ThreadUtils.assertOnUiThread(ThreadUtils.java:203)
04-17 23:51:33.240 10325 10337 W System.err:    at org.chromium.base.memory.MemoryPressureMonitor.getLastReportedPressure(MemoryPressureMonitor.java:194)
04-17 23:51:33.240 10693 10705 E JavaBinder: *** Uncaught remote exception!  (Exceptions are not yet supported across processes.)
04-17 23:51:33.240 10693 10705 E JavaBinder: java.lang.AssertionError: Must be called on the UI thread.
04-17 23:51:33.240 10693 10705 E JavaBinder:    at org.chromium.base.ThreadUtils.assertOnUiThread(ThreadUtils.java:203)
04-17 23:51:33.240 10693 10705 E JavaBinder:    at org.chromium.base.memory.MemoryPressureMonitor.getLastReportedPressure(MemoryPressureMonitor.java:194)
04-17 23:51:33.240 10693 10705 E JavaBinder:    at org.chromium.base.process_launcher.ChildProcessService$1.onMemoryPressure(ChildProcessService.java:154)
04-17 23:51:33.240 10693 10705 E JavaBinder:    at org.chromium.base.process_launcher.IChildProcessService$Stub.onTransact(IChildProcessService.java:81)
04-17 23:51:33.240 10693 10705 E JavaBinder:    at android.os.Binder.execTransact(Binder.java:453)
04-17 23:51:33.240 10325 10337 W System.err:    at org.chromium.base.process_launcher.ChildProcessService$1.onMemoryPressure(ChildProcessService.java:154)
04-17 23:51:33.240 10325 10337 W System.err:    at org.chromium.base.process_launcher.IChildProcessService$Stub.onTransact(IChildProcessService.java:81)
04-17 23:51:33.240 10325 10337 W System.err:    at android.os.Binder.execTransact(Binder.java:453)
04-17 23:51:33.240 10325 10337 E JavaBinder: Forcefully exiting
04-17 23:51:33.240 10693 10705 W System.err: java.lang.AssertionError: Must be called on the UI thread.
04-17 23:51:33.240 10693 10705 W System.err:    at org.chromium.base.ThreadUtils.assertOnUiThread(ThreadUtils.java:203)
04-17 23:51:33.240 10693 10705 W System.err:    at org.chromium.base.memory.MemoryPressureMonitor.getLastReportedPressure(MemoryPressureMonitor.java:194)
04-17 23:51:33.240 10693 10705 W System.err:    at org.chromium.base.process_launcher.ChildProcessService$1.onMemoryPressure(ChildProcessService.java:154)
04-17 23:51:33.240 10693 10705 W System.err:    at org.chromium.base.process_launcher.IChildProcessService$Stub.onTransact(IChildProcessService.java:81)
04-17 23:51:33.240 10693 10705 W System.err:    at android.os.Binder.execTransact(Binder.java:453)

 

Comment 1 by dskiba@chromium.org, Apr 17 2018

(facepalm) I'm on it...

Comment 2 by dskiba@chromium.org, Apr 17 2018

Status: Started (was: Untriaged)

Comment 3 by dskiba@chromium.org, Apr 18 2018

Hmm, so does this mean QC runs tests in release mode?

Comment 4 by boliu@chromium.org, Apr 18 2018

No, I think this line means the exception was basically ignored:
04-17 23:51:33.240 10325 10337 E JavaBinder: *** Uncaught remote exception!  (Exceptions are not yet supported across processes.)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 18 2018

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

commit 75d33ec672525f32725f668c848e7f98c1503d1e
Author: Dmitry Skiba <dskiba@chromium.org>
Date: Wed Apr 18 18:42:33 2018

Fix usage of MemoryPressureMonitor from non-UI thread.

MemoryPressureMonitor is supposed to be called only from UI thread
(as defined by ThreadUtils), but was called from a binder thread by
ChildProcessService$1.onMemoryPressure (see the bug).

The issue was introduced because MemoryPressureMonitor.notifyPressure()
is an exception from MemoryPressureMonitor "UI thread only" rule, and
can be called from any thread. When onMemoryPressure() was initially
added, it had just a single notifyPressure() call, so no posting to UI
thread was necessary. Later getLastReportedPressure() call was added,
but issue with calling it on non-UI thread was overlooked.

This CL fixes the original issue, and also removes notifyPressure()
special case, avoiding similar threading mistakes in the future.

Bug:  834099 
Change-Id: Idc5cc478eac217f2c49d642187677e9bf7a5309f
Reviewed-on: https://chromium-review.googlesource.com/1016095
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Dmitry Skiba <dskiba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551760}
[modify] https://crrev.com/75d33ec672525f32725f668c848e7f98c1503d1e/android_webview/glue/java/src/com/android/webview/chromium/SharedStatics.java
[modify] https://crrev.com/75d33ec672525f32725f668c848e7f98c1503d1e/base/android/java/src/org/chromium/base/memory/MemoryPressureMonitor.java
[modify] https://crrev.com/75d33ec672525f32725f668c848e7f98c1503d1e/base/android/java/src/org/chromium/base/process_launcher/ChildProcessService.java
[modify] https://crrev.com/75d33ec672525f32725f668c848e7f98c1503d1e/base/android/junit/src/org/chromium/base/memory/MemoryPressureMonitorTest.java

Comment 6 by dskiba@chromium.org, Apr 19 2018

Status: Fixed (was: Started)

Comment 7 by boliu@chromium.org, Apr 19 2018

does this need to be merged? log spam is pretty annoying too

Sign in to add a comment