From 722533f283ef642e76ca6f0df99b69e40af75c16 Mon Sep 17 00:00:00 2001 From: satayev Date: Tue, 21 Dec 2021 18:44:36 +0000 Subject: [PATCH] Ignore requests to start an already started system service. For apex-system-services, we allow them to be explicitly started in SystemService if there is a dependency on them from the rest of the framework during dessert development. We also need to preserve their manifest declaration for any previously launched dessert releases. To avoid starting the same service multiple times, keep track of which services have been started in SystemServiceManager and ignore duplicates. See go/updatable-system-services#bookmark=id.dnv20ia7siqd for more details. Bug: 192880996 Test: presubmit Change-Id: I0a9e48eac9fd5739e2c35818fa483a8e479c13a1 --- .../android/server/SystemServiceManager.java | 14 +++++++ .../java/com/android/server/SystemServer.java | 1 - .../server/SystemServiceManagerTest.java | 39 ++++++++++++++++--- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/SystemServiceManager.java b/services/core/java/com/android/server/SystemServiceManager.java index 5f295ac8f20d5..e7f4de2a05883 100644 --- a/services/core/java/com/android/server/SystemServiceManager.java +++ b/services/core/java/com/android/server/SystemServiceManager.java @@ -26,6 +26,7 @@ import android.os.Environment; import android.os.SystemClock; import android.os.Trace; import android.os.UserHandle; +import android.util.ArraySet; import android.util.EventLog; import android.util.IndentingPrintWriter; import android.util.Slog; @@ -47,6 +48,7 @@ import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -94,6 +96,7 @@ public final class SystemServiceManager implements Dumpable { // Services that should receive lifecycle events. private List mServices; + private Set mServiceClassnames; private int mCurrentPhase = -1; @@ -116,6 +119,7 @@ public final class SystemServiceManager implements Dumpable { SystemServiceManager(Context context) { mContext = context; mServices = new ArrayList<>(); + mServiceClassnames = new ArraySet<>(); // Disable using the thread pool for low ram devices sUseLifecycleThreadPool = sUseLifecycleThreadPool && !ActivityManager.isLowRamDeviceStatic(); @@ -210,8 +214,17 @@ public final class SystemServiceManager implements Dumpable { } public void startService(@NonNull final SystemService service) { + // Check if already started + String className = service.getClass().getName(); + if (mServiceClassnames.contains(className)) { + Slog.i(TAG, "Not starting an already started service " + className); + return; + } + mServiceClassnames.add(className); + // Register it. mServices.add(service); + // Start it. long time = SystemClock.elapsedRealtime(); try { @@ -225,6 +238,7 @@ public final class SystemServiceManager implements Dumpable { /** Disallow starting new services after this call. */ void sealStartedServices() { + mServiceClassnames = Collections.emptySet(); mServices = Collections.unmodifiableList(mServices); } diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java index 4673a6905fc01..e0b8672a0ac06 100644 --- a/services/java/com/android/server/SystemServer.java +++ b/services/java/com/android/server/SystemServer.java @@ -3053,7 +3053,6 @@ public final class SystemServer implements Dumpable { private void startApexServices(@NonNull TimingsTraceAndSlog t) { t.traceBegin("startApexServices"); Map services = ApexManager.getInstance().getApexSystemServices(); - // TODO(satayev): filter out already started services // TODO(satayev): introduce android:order for services coming the same apexes for (String name : new TreeSet<>(services.keySet())) { String jarPath = services.get(name); diff --git a/services/tests/servicestests/src/com/android/server/SystemServiceManagerTest.java b/services/tests/servicestests/src/com/android/server/SystemServiceManagerTest.java index 4413dc8b97343..f92f5ead2c3b3 100644 --- a/services/tests/servicestests/src/com/android/server/SystemServiceManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/SystemServiceManagerTest.java @@ -23,6 +23,7 @@ import android.test.AndroidTestCase; import org.junit.Test; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; /** @@ -32,24 +33,52 @@ public class SystemServiceManagerTest extends AndroidTestCase { private static final String TAG = "SystemServiceManagerTest"; + private final SystemServiceManager mSystemServiceManager = + new SystemServiceManager(getContext()); + @Test public void testSealStartedServices() throws Exception { - SystemServiceManager manager = new SystemServiceManager(getContext()); // must be effectively final, since it's changed from inner class below AtomicBoolean serviceStarted = new AtomicBoolean(false); - SystemService service = new SystemService(getContext()) { + SystemService service1 = new SystemService(getContext()) { @Override public void onStart() { serviceStarted.set(true); } }; + SystemService service2 = new SystemService(getContext()) { + @Override + public void onStart() { + throw new IllegalStateException("Second service must not be called"); + } + }; // started services have their #onStart methods called - manager.startService(service); + mSystemServiceManager.startService(service1); assertTrue(serviceStarted.get()); // however, after locking started services, it is not possible to start a new service - manager.sealStartedServices(); - assertThrows(UnsupportedOperationException.class, () -> manager.startService(service)); + mSystemServiceManager.sealStartedServices(); + assertThrows(UnsupportedOperationException.class, + () -> mSystemServiceManager.startService(service2)); } + + @Test + public void testDuplicateServices() throws Exception { + AtomicInteger counter = new AtomicInteger(0); + SystemService service = new SystemService(getContext()) { + @Override + public void onStart() { + counter.incrementAndGet(); + } + }; + + mSystemServiceManager.startService(service); + assertEquals(1, counter.get()); + + // manager does not start the same service twice + mSystemServiceManager.startService(service); + assertEquals(1, counter.get()); + } + }