Merge "Ignore requests to start an already started system service."

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