From cd8f5d8b18fdd66ebb992b2a6ee3a587b6fd5e42 Mon Sep 17 00:00:00 2001 From: Tomislav Novak Date: Wed, 3 May 2023 14:36:51 -0700 Subject: [PATCH] Ignore BIND_ABOVE_CLIENT for same-process connections Binding to a service using BIND_ABOVE_CLIENT affects the OOM adjustment of both the client and the service; client's value is dropped by one level in modifyRawOomAdj() to make sure it's lower than the service's. Doing this unconditionally, however, means that the process' OOM score will be reduced if binding to a service that lives in the same process as the client. For example, a top app would have its oom_score_adj set to 100 (visible) rather than 0 (foreground). Test: atest MockingOomAdjusterTests Signed-off-by: Tomislav Novak Merged-In: Iee5024b8f13771bc98a39a20e5f96a89a4c79b4e Change-Id: Iee5024b8f13771bc98a39a20e5f96a89a4c79b4e --- .../com/android/server/am/ActiveServices.java | 4 +++- .../server/am/ProcessServiceRecord.java | 3 ++- .../server/am/MockingOomAdjusterTests.java | 23 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index c36e0700c723b..7aef89452c787 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -2928,7 +2928,9 @@ public final class ActiveServices { final ProcessServiceRecord clientPsr = b.client.mServices; clientPsr.addConnection(c); c.startAssociationIfNeeded(); - if ((c.flags&Context.BIND_ABOVE_CLIENT) != 0) { + // Don't set hasAboveClient if binding to self to prevent modifyRawOomAdj() from + // dropping the process' adjustment level. + if (b.client != s.app && (c.flags & Context.BIND_ABOVE_CLIENT) != 0) { clientPsr.setHasAboveClient(true); } if ((c.flags&Context.BIND_ALLOW_WHITELIST_MANAGEMENT) != 0) { diff --git a/services/core/java/com/android/server/am/ProcessServiceRecord.java b/services/core/java/com/android/server/am/ProcessServiceRecord.java index 67eb675503ad3..31830c5f6ee3f 100644 --- a/services/core/java/com/android/server/am/ProcessServiceRecord.java +++ b/services/core/java/com/android/server/am/ProcessServiceRecord.java @@ -258,7 +258,8 @@ final class ProcessServiceRecord { mHasAboveClient = false; for (int i = mConnections.size() - 1; i >= 0; i--) { ConnectionRecord cr = mConnections.valueAt(i); - if ((cr.flags & Context.BIND_ABOVE_CLIENT) != 0) { + if (cr.binding.service.app.mServices != this + && (cr.flags & Context.BIND_ABOVE_CLIENT) != 0) { mHasAboveClient = true; break; } diff --git a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java index 8d59dce29bd90..9397533bdf8df 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java @@ -64,6 +64,7 @@ import static com.android.server.am.ProcessList.UNKNOWN_ADJ; import static com.android.server.am.ProcessList.VISIBLE_APP_ADJ; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalAnswers.answer; @@ -2208,6 +2209,28 @@ public class MockingOomAdjusterTests { assertProcStates(app2, false, PROCESS_STATE_SERVICE, SERVICE_ADJ, "started-services"); } + @SuppressWarnings("GuardedBy") + @Test + public void testUpdateOomAdj_DoOne_AboveClient_SameProcess() { + ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID, + MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true)); + doReturn(PROCESS_STATE_TOP).when(sService.mAtmInternal).getTopProcessState(); + doReturn(app).when(sService).getTopApp(); + sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE); + sService.mOomAdjuster.updateOomAdjLocked(app, OomAdjuster.OOM_ADJ_REASON_NONE); + + assertEquals(FOREGROUND_APP_ADJ, app.mState.getSetAdj()); + + // Simulate binding to a service in the same process using BIND_ABOVE_CLIENT and + // verify that its OOM adjustment level is unaffected. + bindService(app, app, null, Context.BIND_ABOVE_CLIENT, mock(IBinder.class)); + app.mServices.updateHasAboveClientLocked(); + assertFalse(app.mServices.hasAboveClient()); + + sService.mOomAdjuster.updateOomAdjLocked(app, OomAdjuster.OOM_ADJ_REASON_NONE); + assertEquals(FOREGROUND_APP_ADJ, app.mState.getSetAdj()); + } + private ProcessRecord makeDefaultProcessRecord(int pid, int uid, String processName, String packageName, boolean hasShownUi) { long now = SystemClock.uptimeMillis();