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()); + } + }