diff --git a/services/core/java/com/android/server/SystemServiceManager.java b/services/core/java/com/android/server/SystemServiceManager.java index 1208cb7e3813b..5f295ac8f20d5 100644 --- a/services/core/java/com/android/server/SystemServiceManager.java +++ b/services/core/java/com/android/server/SystemServiceManager.java @@ -45,6 +45,8 @@ import java.io.File; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -91,7 +93,7 @@ public final class SystemServiceManager implements Dumpable { private long mRuntimeStartUptime; // Services that should receive lifecycle events. - private final ArrayList mServices = new ArrayList(); + private List mServices; private int mCurrentPhase = -1; @@ -113,11 +115,12 @@ public final class SystemServiceManager implements Dumpable { SystemServiceManager(Context context) { mContext = context; + mServices = new ArrayList<>(); // Disable using the thread pool for low ram devices sUseLifecycleThreadPool = sUseLifecycleThreadPool - && !ActivityManager.isLowRamDeviceStatic(); + && !ActivityManager.isLowRamDeviceStatic(); mNumUserPoolThreads = Math.min(Runtime.getRuntime().availableProcessors(), - DEFAULT_MAX_USER_POOL_THREADS); + DEFAULT_MAX_USER_POOL_THREADS); } /** @@ -220,11 +223,16 @@ public final class SystemServiceManager implements Dumpable { warnIfTooLong(SystemClock.elapsedRealtime() - time, service, "onStart"); } + /** Disallow starting new services after this call. */ + void sealStartedServices() { + mServices = Collections.unmodifiableList(mServices); + } + /** * Starts the specified boot phase for all system services that have been started up to * this point. * - * @param t trace logger + * @param t trace logger * @param phase The boot phase to start. */ public void startBootPhase(@NonNull TimingsTraceAndSlog t, int phase) { @@ -398,8 +406,8 @@ public final class SystemServiceManager implements Dumpable { // Limit the lifecycle parallelization to all users other than the system user // and only for the user start lifecycle phase for now. final boolean useThreadPool = sUseLifecycleThreadPool - && curUserId != UserHandle.USER_SYSTEM - && onWhat.equals(USER_STARTING); + && curUserId != UserHandle.USER_SYSTEM + && onWhat.equals(USER_STARTING); final ExecutorService threadPool = useThreadPool ? Executors.newFixedThreadPool(mNumUserPoolThreads) : null; for (int i = 0; i < serviceLen; i++) { @@ -419,7 +427,7 @@ public final class SystemServiceManager implements Dumpable { + serviceName + " because it's not supported (curUser: " + curUser + ", prevUser:" + prevUser + ")"); } else { - Slog.i(TAG, "Skipping " + onWhat + "User-" + curUserId + " on " + Slog.i(TAG, "Skipping " + onWhat + "User-" + curUserId + " on " + serviceName); } continue; @@ -516,6 +524,7 @@ public final class SystemServiceManager implements Dumpable { /** * Returns whether we are booting into safe mode. + * * @return safe mode flag */ public boolean isSafeMode() { @@ -559,9 +568,10 @@ public final class SystemServiceManager implements Dumpable { /** * Ensures that the system directory exist creating one if needed. + * + * @return The system directory. * @deprecated Use {@link Environment#getDataSystemCeDirectory()} * or {@link Environment#getDataSystemDeDirectory()} instead. - * @return The system directory. */ @Deprecated public static File ensureSystemDir() { @@ -578,7 +588,9 @@ public final class SystemServiceManager implements Dumpable { pw.printf("Current phase: %d\n", mCurrentPhase); synchronized (mTargetUsers) { if (mCurrentUser != null) { - pw.print("Current user: "); mCurrentUser.dump(pw); pw.println(); + pw.print("Current user: "); + mCurrentUser.dump(pw); + pw.println(); } else { pw.println("Current user not set!"); } diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java index 29797a549e493..4673a6905fc01 100644 --- a/services/java/com/android/server/SystemServer.java +++ b/services/java/com/android/server/SystemServer.java @@ -920,12 +920,6 @@ public final class SystemServer implements Dumpable { startBootstrapServices(t); startCoreServices(t); startOtherServices(t); - // Apex services must be the last category of services to start. No other service must - // be starting after this point. This is to prevent unnessary stability issues when - // these apexes are updated outside of OTA; and to avoid breaking dependencies from - // system into apexes. - // TODO(satayev): lock mSystemServiceManager.startService to stop accepting new services - // after this step startApexServices(t); } catch (Throwable ex) { Slog.e("System", "******************************************"); @@ -3051,6 +3045,10 @@ public final class SystemServer implements Dumpable { /** * Starts system services defined in apexes. + * + *

Apex services must be the last category of services to start. No other service must be + * starting after this point. This is to prevent unnecessary stability issues when these apexes + * are updated outside of OTA; and to avoid breaking dependencies from system into apexes. */ private void startApexServices(@NonNull TimingsTraceAndSlog t) { t.traceBegin("startApexServices"); @@ -3067,6 +3065,10 @@ public final class SystemServer implements Dumpable { } t.traceEnd(); } + + // make sure no other services are started after this point + mSystemServiceManager.sealStartedServices(); + t.traceEnd(); // startApexServices } diff --git a/services/tests/servicestests/src/com/android/server/SystemServiceManagerTest.java b/services/tests/servicestests/src/com/android/server/SystemServiceManagerTest.java new file mode 100644 index 0000000000000..4413dc8b97343 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/SystemServiceManagerTest.java @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2021 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; + +import static org.junit.Assert.assertThrows; + +import android.test.AndroidTestCase; + +import org.junit.Test; + +import java.util.concurrent.atomic.AtomicBoolean; + + +/** + * Tests for {@link SystemServiceManager}. + */ +public class SystemServiceManagerTest extends AndroidTestCase { + + private static final String TAG = "SystemServiceManagerTest"; + + @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()) { + @Override + public void onStart() { + serviceStarted.set(true); + } + }; + + // started services have their #onStart methods called + manager.startService(service); + 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)); + } +}