From 7e72d90e58ad0c5ad6695bac1951eb0afe89423d Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 25 Oct 2022 16:31:08 -0600 Subject: [PATCH] BroadcastQueue: make "oneway" calls non-blocking. When an IApplicationThread instance is hosted by the system_server process, our various schedule-broadcast calls are dispatched as blocking methods (with the AMS lock held!!), instead of matching the expected "oneway" contract. This change offers ProcessRecord.getOnewayThread() which dispatches calls through FgThread using SameProcessApplicationThread, which starts with initial support for broadcast-related methods, but could be expanded in the future where needed. Bug: 255532202 Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Change-Id: I7fb2a7353c44c29cda94e00fc87b8cff6ffc0008 --- .../server/am/BroadcastProcessQueue.java | 2 +- .../server/am/BroadcastQueueModernImpl.java | 4 +- .../com/android/server/am/ProcessRecord.java | 19 +++++ .../am/SameProcessApplicationThread.java | 73 +++++++++++++++++++ .../am/BroadcastQueueModernImplTest.java | 4 + .../android/server/am/BroadcastQueueTest.java | 4 +- 6 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 services/core/java/com/android/server/am/SameProcessApplicationThread.java diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java index 093c7b510d776..9ec1fb42ce5a8 100644 --- a/services/core/java/com/android/server/am/BroadcastProcessQueue.java +++ b/services/core/java/com/android/server/am/BroadcastProcessQueue.java @@ -349,7 +349,7 @@ class BroadcastProcessQueue { * Return if we know of an actively running "warm" process for this queue. */ public boolean isProcessWarm() { - return (app != null) && (app.getThread() != null) && !app.isKilled(); + return (app != null) && (app.getOnewayThread() != null) && !app.isKilled(); } public int getPreferredSchedulingGroupLocked() { diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index 6b0bbbe003888..2e662b4117f3f 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -736,7 +736,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { if (DEBUG_BROADCAST) logv("Scheduling " + r + " to warm " + app); setDeliveryState(queue, app, r, index, receiver, BroadcastRecord.DELIVERY_SCHEDULED); - final IApplicationThread thread = app.getThread(); + final IApplicationThread thread = app.getOnewayThread(); if (thread != null) { try { if (receiver instanceof BroadcastFilter) { @@ -777,7 +777,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private void scheduleResultTo(@NonNull BroadcastRecord r) { if ((r.resultToApp == null) || (r.resultTo == null)) return; final ProcessRecord app = r.resultToApp; - final IApplicationThread thread = app.getThread(); + final IApplicationThread thread = app.getOnewayThread(); if (thread != null) { mService.mOomAdjuster.mCachedAppOptimizer.unfreezeTemporarily( app, OOM_ADJ_REASON_FINISH_RECEIVER); diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index 3b04dbb1da98d..0a8c6400a6fd0 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -54,6 +54,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.procstats.ProcessState; import com.android.internal.app.procstats.ProcessStats; import com.android.internal.os.Zygote; +import com.android.server.FgThread; import com.android.server.wm.WindowProcessController; import com.android.server.wm.WindowProcessListener; @@ -142,6 +143,13 @@ class ProcessRecord implements WindowProcessListener { @CompositeRWLock({"mService", "mProcLock"}) private IApplicationThread mThread; + /** + * Instance of {@link #mThread} that will always meet the {@code oneway} + * contract, possibly by using {@link SameProcessApplicationThread}. + */ + @CompositeRWLock({"mService", "mProcLock"}) + private IApplicationThread mOnewayThread; + /** * Always keep this application running? */ @@ -603,16 +611,27 @@ class ProcessRecord implements WindowProcessListener { return mThread; } + @GuardedBy(anyOf = {"mService", "mProcLock"}) + IApplicationThread getOnewayThread() { + return mOnewayThread; + } + @GuardedBy({"mService", "mProcLock"}) public void makeActive(IApplicationThread thread, ProcessStatsService tracker) { mProfile.onProcessActive(thread, tracker); mThread = thread; + if (mPid == Process.myPid()) { + mOnewayThread = new SameProcessApplicationThread(thread, FgThread.getHandler()); + } else { + mOnewayThread = thread; + } mWindowProcessController.setThread(thread); } @GuardedBy({"mService", "mProcLock"}) public void makeInactive(ProcessStatsService tracker) { mThread = null; + mOnewayThread = null; mWindowProcessController.setThread(null); mProfile.onProcessInactive(tracker); } diff --git a/services/core/java/com/android/server/am/SameProcessApplicationThread.java b/services/core/java/com/android/server/am/SameProcessApplicationThread.java new file mode 100644 index 0000000000000..a3c011188539d --- /dev/null +++ b/services/core/java/com/android/server/am/SameProcessApplicationThread.java @@ -0,0 +1,73 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.am; + +import android.annotation.NonNull; +import android.app.IApplicationThread; +import android.content.IIntentReceiver; +import android.content.Intent; +import android.content.pm.ActivityInfo; +import android.content.res.CompatibilityInfo; +import android.os.Bundle; +import android.os.Handler; +import android.os.RemoteException; + +import java.util.Objects; + +/** + * Wrapper around an {@link IApplicationThread} that delegates selected calls + * through a {@link Handler} so they meet the {@code oneway} contract of + * returning immediately after dispatch. + */ +public class SameProcessApplicationThread extends IApplicationThread.Default { + private final IApplicationThread mWrapped; + private final Handler mHandler; + + public SameProcessApplicationThread(@NonNull IApplicationThread wrapped, + @NonNull Handler handler) { + mWrapped = Objects.requireNonNull(wrapped); + mHandler = Objects.requireNonNull(handler); + } + + @Override + public void scheduleReceiver(Intent intent, ActivityInfo info, CompatibilityInfo compatInfo, + int resultCode, String data, Bundle extras, boolean sync, int sendingUser, + int processState) { + mHandler.post(() -> { + try { + mWrapped.scheduleReceiver(intent, info, compatInfo, resultCode, data, extras, sync, + sendingUser, processState); + } catch (RemoteException e) { + throw new RuntimeException(e); + } + }); + } + + @Override + public void scheduleRegisteredReceiver(IIntentReceiver receiver, Intent intent, int resultCode, + String data, Bundle extras, boolean ordered, boolean sticky, int sendingUser, + int processState) { + mHandler.post(() -> { + try { + mWrapped.scheduleRegisteredReceiver(receiver, intent, resultCode, data, extras, + ordered, sticky, sendingUser, processState); + } catch (RemoteException e) { + throw new RuntimeException(e); + } + }); + } +} diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java index ba414cb593efe..ee3815428b25b 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java @@ -87,6 +87,10 @@ public class BroadcastQueueModernImplTest { mHandlerThread.start(); mConstants = new BroadcastConstants(Settings.Global.BROADCAST_FG_CONSTANTS); + mConstants.DELAY_URGENT_MILLIS = -120_000; + mConstants.DELAY_NORMAL_MILLIS = 10_000; + mConstants.DELAY_CACHED_MILLIS = 120_000; + mImpl = new BroadcastQueueModernImpl(mAms, mHandlerThread.getThreadHandler(), mConstants, mConstants); diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java index 0a428c415e4de..e1a4c1dd72566 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java @@ -897,8 +897,8 @@ public class BroadcastQueueTest { // cold-started apps to be thawed, but the modern stack does } else { // Confirm that app was thawed - verify(mAms.mOomAdjuster.mCachedAppOptimizer).unfreezeTemporarily(eq(receiverApp), - eq(OomAdjuster.OOM_ADJ_REASON_START_RECEIVER)); + verify(mAms.mOomAdjuster.mCachedAppOptimizer, atLeastOnce()).unfreezeTemporarily( + eq(receiverApp), eq(OomAdjuster.OOM_ADJ_REASON_START_RECEIVER)); // Confirm that we added package to process verify(receiverApp, atLeastOnce()).addPackage(eq(receiverApp.info.packageName),