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

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<SystemService> mServices;
+    private Set<String> 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);
     }
 

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<String, String> 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);

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