Skip to content

add a system_server API for listening to GosPackageState changes; fix racy dispatch of GosPackageState change callback #406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
muhomorr wants to merge 10 commits into
base: 17
Choose a base branch
from
Open

add a system_server API for listening to GosPackageState changes; fix racy dispatch of GosPackageState change callback #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
830db49
fixup! SystemUI: require unlocking to use QS tiles by default
muhomorr Jul 10, 2026
7fbb368
gosps: fix racy dispatch of GosPackageState change callback
muhomorr Jul 9, 2026
bc0e6ff
gosps: convert GosPackageStatePmHooks into an object instance
muhomorr Jul 10, 2026
460c011
gosps: add toString() implementation for GosPackageState
muhomorr Jul 10, 2026
9fd6941
gosps: use a more descriptive reason for AMS.killUid()
muhomorr Jul 10, 2026
ee07d09
gosps: add a system_server API for listening to GosPackageState changes
muhomorr Jul 10, 2026
a8f226e
gosps: don't rely on ActivityThread.mInitialApplication
muhomorr Jul 11, 2026
7163a0b
gosps: switch changeCallbacks from ArrayList to CopyOnWriteArrayList
muhomorr Jul 11, 2026
8ca4439
gosps: remove the dedicated GosPackageStateChangeCallbacks thread
muhomorr Jul 11, 2026
88ccffe
gosps: update onGosPackageStateChanged() javadoc
muhomorr Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions core/java/android/app/ActivityManagerInternal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import android.content.pm.ActivityInfo;
import android.content.pm.ActivityPresentationInfo;
import android.content.pm.ApplicationInfo;
import android.content.pm.GosPackageState;
import android.content.pm.IPackageDataObserver;
import android.content.pm.UserInfo;
import android.net.Uri;
Expand Down Expand Up @@ -1478,7 +1477,7 @@ public abstract void getExecutableMethodFileOffsets(@NonNull String processName,
*/
public abstract boolean hasServiceBindingOrProviderUse(int uid, int clientUid);

public abstract void onGosPackageStateChanged(int uid, GosPackageState state);
public abstract void dispatchGosPackageStateCallbacks(int uid);

public static class ProcessRecordSnapshot {
public final int pid;
Expand Down
6 changes: 2 additions & 4 deletions core/java/android/app/ActivityThread.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@
import android.content.pm.ActivityInfo;
import android.content.pm.ApplicationInfo;
import android.content.pm.ComponentInfo;
import android.content.pm.GosPackageState;
import android.content.pm.IPackageManager;
import android.content.pm.InstrumentationInfo;
import android.content.pm.PackageInfo;
Expand Down Expand Up @@ -2517,9 +2516,8 @@ public void getExecutableMethodFileOffsets(
}

@Override
public void onGosPackageStateChanged(GosPackageState state) {
// this is a oneway method, caller (ActivityManager) will not be blocked
ActivityThreadHooks.onGosPackageStateChanged(mInitialApplication, state, false);
public void onGosPackageStateChanged() {
ActivityThreadHooks.onGosPackageStateChanged(null);
}
Comment on lines 2518 to 2521

@inthewaves inthewaves Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a window where mInitialApplication can be null, e.g. if an app overrides Application#attachBaseContext and takes too long, and then storage/contact scopes is enabled during that time.

There seems to already be AOSP code here that accounts for this, e.g. see how mUpdateHttpProxyOnBind is used in ActivityThread#updateHttpProxy, etc.

e.g. as a (contrived) NPE example, consider an app that has this Application:

class SlowStartApplication : Application() {
    override fun attachBaseContext(base: Context) {
        super.attachBaseContext(base)

        Log.i(TAG, "attachBaseContext: blocking startup for ${STARTUP_BLOCK_MS}ms")
        Log.i(TAG, "attachBaseContext: enable Storage Scopes or Contact Scopes while this process is blocked")
        SystemClock.sleep(STARTUP_BLOCK_MS)
        Log.i(TAG, "attachBaseContext: startup block finished")
    }

    override fun onCreate() {
        super.onCreate()
        Log.i(TAG, "onCreate: application startup completed")
    }

    companion object {
        const val TAG = "SampleApp"
        private const val STARTUP_BLOCK_MS = 12_000L
    }
}

Then attachBaseContext's blocking/slow code can be triggered by the app receiving a broadcast, so it's not always from a user launching the app then going to configure storage/contact scopes. The app can receive a broadcast and run its application code while the user is turning on storage/contact scopes

@inthewaves inthewaves Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NPE when such an app receives broadcast to run its slow attachBaseContext code and Storage Scopes is enabled while it's blocked:

 6933-6933  SampleApp                I  attachBaseContext: blocking startup for 12000ms
 6933-6933  SampleApp                I  attachBaseContext: enable Storage Scopes or Contact Scopes while this process is blocked
 6933-6947  Binder                   W  Caught a RuntimeException from the binder stub implementation.
                                        java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String android.content.Context.getPackageName()' on a null object reference
                                        	at android.content.pm.GosPackageState.getForSelf(GosPackageState.java:244)
                                        	at android.app.ActivityThread$ApplicationThread.onGosPackageStateChanged(ActivityThread.java:2522)
                                        	at android.app.IApplicationThread$Stub.onTransact(IApplicationThread.java:1595)
                                        	at android.app.ActivityThread$ApplicationThread.onTransact(ActivityThread.java:1248)
                                        	at android.os.Binder.execTransactInternal(Binder.java:1417)
                                        	at android.os.Binder.execTransact(Binder.java:1364)
 6933-6933  SampleApp                I  attachBaseContext: startup block finished
 6933-6933  SampleApp                I  onCreate: application startup completed

NPE can also be triggered in the old code with Contact Scopes since that uses the ctx

@muhomorr muhomorr Jul 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a8f226e

}

Expand Down
63 changes: 49 additions & 14 deletions core/java/android/app/ActivityThreadHooks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package android.app;

import android.annotation.Nullable;
import android.content.Context;
import android.content.pm.GosPackageState;
import android.content.pm.SrtPermissions;
Expand All @@ -8,27 +9,28 @@
import android.location.HookedLocationManager;
import android.os.Bundle;
import android.os.Process;
import android.util.Log;

import com.android.internal.app.ContactScopes;
import com.android.internal.app.StorageScopesAppHooks;
import com.android.internal.gmscompat.GmsHooks;
import com.android.internal.util.Preconditions;

import java.util.Objects;

class ActivityThreadHooks {

private static volatile boolean called;
@Nullable // null during the early part of app process init
private static volatile Context appContext;
private static volatile boolean onBindCalled;

// called after the initial app context is constructed
// ActivityThread.handleBindApplication
static Bundle onBind(ActivityThread.AppBindData appBindData) {
Bundle args = appBindData.extraArgs;
Objects.requireNonNull(args, "args bundle is null");

if (called) {
throw new IllegalStateException("onBind called for the second time");
}
called = true;
Preconditions.checkState(!onBindCalled);
onBindCalled = true;

AssetManager.systemIdmapPaths_ = args.getStringArray(AppBindArgs.KEY_SYSTEM_IDMAP_PATHS);

Expand All @@ -49,17 +51,50 @@ static Bundle onBind(ActivityThread.AppBindData appBindData) {
// of app's code
// ActivityThread.handleBindApplication
static void onBind2(Context appContext, Bundle appBindArgs) {
GosPackageState gosPs = appBindArgs.getParcelable(AppBindArgs.KEY_GOS_PACKAGE_STATE,
GosPackageState.class);
onGosPackageStateChanged(appContext, gosPs, true);
ActivityThreadHooks.appContext = appContext;
if (!Process.isIsolated()) { // isolated processes don't have access to GosPackageState
onGosPackageStateChanged(appBindArgs);
}
}

// called from both main and worker threads
static void onGosPackageStateChanged(Context ctx, GosPackageState state, boolean fromBind) {
if (!Process.isIsolated()) {
StorageScopesAppHooks.maybeEnable(state);
ContactScopes.maybeEnable(ctx, state);
private static final Object gosPackageStateChangeLock = new Object();
private static boolean hadInitialGosPsChangeCallback;
private static boolean hasPendingGosPsChangeCallback;

// called from both main and binder threads
static void onGosPackageStateChanged(@Nullable Bundle appBindArgs) {
Context ctx = appContext;
GosPackageState state;
synchronized (gosPackageStateChangeLock) {
if (appBindArgs != null) {
Preconditions.checkState(!hadInitialGosPsChangeCallback);
hadInitialGosPsChangeCallback = true;
// app context is always initialized by this point
Objects.requireNonNull(ctx);
if (hasPendingGosPsChangeCallback) {
Log.i("GosPackageState", "ignoring AppBindArgs since hasPendingGosPsChangeCallback is set");
}
state = hasPendingGosPsChangeCallback ?
// GosPackageState from AppBindArgs is outdated, obtain a fresh one
GosPackageState.getForSelf(ctx) :
// this is the initial onGosPackageStateChanged() call during app binding,
// use GosPackageState from AppBindArgs to avoid IPC
appBindArgs.getParcelable(AppBindArgs.KEY_GOS_PACKAGE_STATE, GosPackageState.class);

Objects.requireNonNull(state);
hasPendingGosPsChangeCallback = false;
} else {
if (ctx == null) {
Preconditions.checkState(!hadInitialGosPsChangeCallback);
hasPendingGosPsChangeCallback = true;
Log.i("GosPackageState", "set hasPendingGosPsChangeCallback");
return;
}
state = GosPackageState.getForSelf(ctx);
}
}
StorageScopesAppHooks.maybeEnable(state);
ContactScopes.maybeEnable(ctx, state);
}

static Service instantiateService(String className) {
Expand Down
2 changes: 1 addition & 1 deletion core/java/android/app/IApplicationThread.aidl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ oneway interface IApplicationThread {
void getExecutableMethodFileOffsets(in MethodDescriptor methodDescriptor,
in IOffsetCallback resultCallback);

void onGosPackageStateChanged(in android.content.pm.GosPackageState state);
void onGosPackageStateChanged();

void requestHandoffActivityData(in IBinder requestToken, in List<IBinder> activityTokens);
}
68 changes: 68 additions & 0 deletions core/java/android/content/pm/GosPackageState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
import android.os.RemoteException;
import android.os.UserHandle;
import android.permission.PermissionManager;
import android.util.Log;
import android.util.SparseArray;

import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.Objects;

Expand Down Expand Up @@ -171,6 +174,71 @@ public boolean isNone() {
return this == NONE;
}

private static volatile SparseArray<String> flagNames;

@Override
public String toString() {
if (this == DEFAULT) {
return "GosPackageState.DEFAULT";
}
if (this == NONE) {
return "GosPackageState.NONE";
}
var b = new StringBuilder("GosPackageState{");
int origLength = b.length();
if (flagStorage1 != 0) {
SparseArray<String> flagNames = GosPackageState.flagNames;
if (flagNames == null) {
Field[] fields = GosPackageStateFlag.class.getDeclaredFields();
int numFields = fields.length;
flagNames = new SparseArray(numFields);
for (int i = 0; i < numFields; ++i) {
Field f = fields[i];
int fieldValue;
try {
fieldValue = (int) f.get(null);
} catch (ReflectiveOperationException e) {
Log.e("GosPackageState", "", e);
continue;
}
flagNames.set(fieldValue, f.getName());
}
GosPackageState.flagNames = flagNames;
}
b.append("flags:");
boolean first = true;
for (int i = 0; i < Long.SIZE; ++i) {
if (hasFlag(i)) {
String name = flagNames.get(i);
if (name == null) {
name = "UNKNOWN_FLAG_" + i;
}
b.append(first ? " " : " | ");
first = false;
b.append(name);
}
}
b.append(", ");
}
if (packageFlagStorage != 0) {
b.append("packageFlags: 0x");
b.append(Long.toHexString(packageFlagStorage));
b.append(", ");
}
if (storageScopes != null) {
b.append("has storageScopes, ");
}
if (contactScopes != null) {
b.append("has contactScopes, ");
}
if (b.length() != origLength) {
// trim trailing comma + space
b.setLength(b.length() - 2);
}
b.append('}');
return b.toString();
}

@NonNull
public static GosPackageState getForSelf(@NonNull Context context) {
return get(context.getPackageName(), context.getUserId());
Expand Down
5 changes: 5 additions & 0 deletions packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/WifiTileTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.android.systemui.qs.tiles.impl.wifi.domain.interactor.WifiTileUserAct
import com.android.systemui.qs.tiles.impl.wifi.domain.model.WifiTileModel
import com.android.systemui.qs.tiles.impl.wifi.ui.mapper.WifiTileMapper
import com.android.systemui.res.R
import com.android.systemui.statusbar.policy.KeyguardStateController
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
Expand All @@ -61,6 +62,7 @@ import org.mockito.kotlin.whenever
class WifiTileTest : SysuiTestCase() {

@Mock private lateinit var host: QSHost
@Mock private lateinit var keyguardStateController: KeyguardStateController
@Mock private lateinit var uiEventLogger: QsEventLogger
@Mock private lateinit var metricsLogger: MetricsLogger
@Mock private lateinit var statusBarStateController: StatusBarStateController
Expand Down Expand Up @@ -101,6 +103,9 @@ class WifiTileTest : SysuiTestCase() {
}
)

whenever(keyguardStateController.isMethodSecure).thenReturn(false)
whenever(host.keyguardStateController).thenReturn(keyguardStateController)

underTest =
WifiTile(
host,
Expand Down
7 changes: 7 additions & 0 deletions services/api/current.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,12 @@ package com.android.server.permission {
package com.android.server.pm {

public interface PackageManagerLocal {
method public void addGosPackageStateChangeCallback(@NonNull com.android.server.pm.PackageManagerLocal.GosPackageStateChangeCallback);
method @Nullable public String maybeOverrideCompilerFilter(@NonNull String, @NonNull com.android.server.pm.pkg.AndroidPackage, @NonNull Object);
method public void onBgDexoptCompleted(@Nullable Object, long);
method public void onBgDexoptProgressUpdate(long, int, int, int);
method public void reconcileSdkData(@Nullable String, @NonNull String, @NonNull java.util.List<String>, int, int, int, @NonNull String, int) throws java.io.IOException;
method public boolean removeGosPackageStateChangeCallback(@NonNull com.android.server.pm.PackageManagerLocal.GosPackageStateChangeCallback);
method @NonNull public com.android.server.pm.PackageManagerLocal.FilteredSnapshot withFilteredSnapshot();
method @NonNull public com.android.server.pm.PackageManagerLocal.FilteredSnapshot withFilteredSnapshot(int, @NonNull android.os.UserHandle);
method @NonNull public com.android.server.pm.PackageManagerLocal.UnfilteredSnapshot withUnfilteredSnapshot();
Expand All @@ -101,6 +103,11 @@ package com.android.server.pm {
method @NonNull public java.util.Map<String, com.android.server.pm.pkg.PackageState> getPackageStates();
}

public abstract static class PackageManagerLocal.GosPackageStateChangeCallback {
ctor public PackageManagerLocal.GosPackageStateChangeCallback(@NonNull android.os.Handler);
method public abstract void onGosPackageStateChanged(int, @NonNull android.content.pm.GosPackageState, int);
}

public static interface PackageManagerLocal.UnfilteredSnapshot extends AutoCloseable {
method public void close();
method @NonNull public com.android.server.pm.PackageManagerLocal.FilteredSnapshot filtered(int, @NonNull android.os.UserHandle);
Expand Down
7 changes: 3 additions & 4 deletions services/core/java/com/android/server/am/ActivityManagerService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@
import android.content.pm.AllowComponentAccessPolicyInfo;
import android.content.pm.ApplicationInfo;
import android.content.pm.ApplicationInfo.HiddenApiEnforcementPolicy;
import android.content.pm.GosPackageState;
import android.content.pm.IPackageDataObserver;
import android.content.pm.IPackageManager;
import android.content.pm.IncrementalStatesInfo;
Expand Down Expand Up @@ -19401,9 +19400,9 @@ public boolean hasServiceBindingOrProviderUse(int uid, int clientUid) {
}

@Override
public void onGosPackageStateChanged(int uid, GosPackageState state) {
synchronized (mProcLock) {
mProcessList.onGosPackageStateChangedLOSP(uid, state);
public void dispatchGosPackageStateCallbacks(int uid) {
synchronized (mGlobalLock) {
mProcessList.dispatchGosPackageStateChangedLOSP(uid);
}
}

Expand Down
6 changes: 3 additions & 3 deletions services/core/java/com/android/server/am/NativeApplicationThreadWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,9 +525,9 @@ public void getExecutableMethodFileOffsets(
@NonNull MethodDescriptor methodDescriptor, @NonNull IOffsetCallback resultCallback) {}

@Override
public void onGosPackageStateChanged(GosPackageState state) {
Slog.e(TAG, "onGosPackageStateChanged");
System.exit(1);
public void onGosPackageStateChanged() {
// native processes run in isolated UIDs, onGosPackageStateChanged() is never called for them
Slog.wtf(TAG, "onGosPackageStateChanged called");
}

@NeverCompile
Expand Down
13 changes: 6 additions & 7 deletions services/core/java/com/android/server/am/ProcessList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3922,22 +3922,21 @@ void handleAllTrustStorageUpdateLOSP() {
}
}

@GuardedBy(anyOf = {"mService", "mProcLock"})
void onGosPackageStateChangedLOSP(int uid, GosPackageState state) {
@GuardedBy("mService")
void dispatchGosPackageStateChangedLOSP(int uid) {
for (int i = mLruProcesses.size() - 1; i >= 0; i--) {
ProcessRecord r = mLruProcesses.get(i);
if (r.uid != uid) {
// isolated and "sdk sandbox" processes are skipped intentionally (they run in
// separate UIDs)
// isolated processes are skipped intentionally (they run in separate UIDs)
continue;
}
final IApplicationThread thread = r.getThread();
if (thread != null) {
try {
thread.onGosPackageStateChanged(state);
thread.onGosPackageStateChanged();
} catch (RemoteException ex) {
Slog.d(TAG, "onGosPackageStateChanged failed; uid " + uid
+ ", processName " + r.info.processName);
Slog.i(TAG, "onGosPackageStateChanged failed; uid " + uid
+ ", processName " + r.info.processName, ex);
}
}
}
Expand Down
Loading