From 033620236aaae30b58f94b33af10a02c752135ad Mon Sep 17 00:00:00 2001 From: Lakshman Annadorai Date: Fri, 11 Nov 2022 14:31:54 -0800 Subject: [PATCH] Skip duplicate USER_SYSTEM start in SystemServiceManager. On Automotive / Headless System User Mode, the system user will be started twice - once from ActivityServiceManager and once from an external/local service that switches the system user to background in HSUM. Previously, checking for the current user on the ActivityServiceManager side helped avoid starting system user twice because the CarServiceHelperService, which resides in the same process, started the system user before ActivityServiceManager and switched the user to the background. But this doesn't work anymore because the order of these calls are non-deteministic. The system user start is moved to CarService, which resides in a separate process (which increases the chances of the non-deterministic event). Avoid starting system user twice by explicitly checking against the list of previously started users on the SystemServiceManager side. On the ActivityServiceManager side, protect against starting home displays, and broadcasting system user start/switch on HSUM by explicitly checking for HSUM. Test: Build and system bootup is okay. Bug: 258532055 Change-Id: Ic0af6b030c08995715dd3195801ed271c690ce4a Merged-In: Ic0af6b030c08995715dd3195801ed271c690ce4a (cherry picked from commit 123aa680d1bd664b481a26748e0d900496edaea7) --- .../android/server/SystemServiceManager.java | 17 +++++++++-- .../server/am/ActivityManagerService.java | 29 +++++++++++-------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/SystemServiceManager.java b/services/core/java/com/android/server/SystemServiceManager.java index 78df983c83f72..166806b31cd19 100644 --- a/services/core/java/com/android/server/SystemServiceManager.java +++ b/services/core/java/com/android/server/SystemServiceManager.java @@ -357,13 +357,24 @@ public final class SystemServiceManager implements Dumpable { * Starts the given user. */ public void onUserStarting(@NonNull TimingsTraceAndSlog t, @UserIdInt int userId) { - EventLog.writeEvent(EventLogTags.SSM_USER_STARTING, userId); - final TargetUser targetUser = newTargetUser(userId); synchronized (mTargetUsers) { + // On Automotive / Headless System User Mode, the system user will be started twice: + // - Once by some external or local service that switches the system user to + // the background. + // - Once by the ActivityManagerService, when the system is marked ready. + // These two events are not synchronized and the order of execution is + // non-deterministic. To avoid starting the system user twice, verify whether + // the system user has already been started by checking the mTargetUsers. + // TODO(b/242195409): this workaround shouldn't be necessary once we move + // the headless-user start logic to UserManager-land. + if (userId == UserHandle.USER_SYSTEM && mTargetUsers.contains(userId)) { + Slog.e(TAG, "Skipping starting system user twice"); + return; + } mTargetUsers.put(userId, targetUser); } - + EventLog.writeEvent(EventLogTags.SSM_USER_STARTING, userId); onUser(t, USER_STARTING, /* prevUser= */ null, targetUser); } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index fde96b96cb9c3..bc083f1ca01e9 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -8163,15 +8163,12 @@ public class ActivityManagerService extends IActivityManager.Stub mBatteryStatsService.noteEvent(BatteryStats.HistoryItem.EVENT_USER_FOREGROUND_START, Integer.toString(currentUserId), currentUserId); - // On Automotive, at this point the system user has already been started and unlocked, - // and some of the tasks we do here have already been done. So skip those in that case. - // TODO(b/132262830, b/203885241): this workdound shouldn't be necessary once we move the - // headless-user start logic to UserManager-land - final boolean bootingSystemUser = currentUserId == UserHandle.USER_SYSTEM; - - if (bootingSystemUser) { - mSystemServiceManager.onUserStarting(t, currentUserId); - } + // On Automotive / Headless System User Mode, at this point the system user has already been + // started and unlocked, and some of the tasks we do here have already been done. So skip + // those in that case. The duplicate system user start is guarded in SystemServiceManager. + // TODO(b/242195409): this workaround shouldn't be necessary once we move the headless-user + // start logic to UserManager-land. + mSystemServiceManager.onUserStarting(t, currentUserId); synchronized (this) { // Only start up encryption-aware persistent apps; once user is @@ -8201,7 +8198,15 @@ public class ActivityManagerService extends IActivityManager.Stub t.traceEnd(); } - if (bootingSystemUser) { + // Some systems - like automotive - will explicitly unlock system user then switch + // to a secondary user. Hence, we don't want to send duplicate broadcasts for + // the system user here. + // TODO(b/242195409): this workaround shouldn't be necessary once we move + // the headless-user start logic to UserManager-land. + final boolean isBootingSystemUser = (currentUserId == UserHandle.USER_SYSTEM) + && !UserManager.isHeadlessSystemUserMode(); + + if (isBootingSystemUser) { t.traceBegin("startHomeOnAllDisplays"); mAtmInternal.startHomeOnAllDisplays(currentUserId, "systemReady"); t.traceEnd(); @@ -8212,7 +8217,7 @@ public class ActivityManagerService extends IActivityManager.Stub t.traceEnd(); - if (bootingSystemUser) { + if (isBootingSystemUser) { t.traceBegin("sendUserStartBroadcast"); final int callingUid = Binder.getCallingUid(); final int callingPid = Binder.getCallingPid(); @@ -8253,7 +8258,7 @@ public class ActivityManagerService extends IActivityManager.Stub mAtmInternal.resumeTopActivities(false /* scheduleIdle */); t.traceEnd(); - if (bootingSystemUser) { + if (isBootingSystemUser) { t.traceBegin("sendUserSwitchBroadcasts"); mUserController.sendUserSwitchBroadcasts(-1, currentUserId); t.traceEnd();