Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d61edebf authored by Songchun Fan's avatar Songchun Fan Committed by Joanne Chung
Browse files

[pm] move RemoteCallbackList.broadcast to handler thread 

Fixing a system server crash because RemoteCallbackList.broadcast cannot
be called by multiple threads simultaneously. Move it to the handler
thread to avoid that.

BUG: 289240865
Test: builds
Test: atest PackageMonitorCallbackHelperTest
Test: atest CtsAppTestCases
Test: atest CtsDevicePolicyTestCases

Change-Id: I7d6b2c282f031953862fe9cac8c3bb7449ac50b0
parent fcb5e2c1

services/core/java/com/android/server/pm/PackageManagerService.java

+3 −2
Original line number Diff line number Diff line
@@ -712,8 +712,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
    private final PackageObserverHelper mPackageObserverHelper = new PackageObserverHelper();



    @NonNull

    private final PackageMonitorCallbackHelper mPackageMonitorCallbackHelper =

            new PackageMonitorCallbackHelper();

    private final PackageMonitorCallbackHelper mPackageMonitorCallbackHelper;



    private final ModuleInfoProvider mModuleInfoProvider;
@@ -1837,6 +1836,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
        mSharedLibraries.setDeletePackageHelper(mDeletePackageHelper);



        mStorageEventHelper = testParams.storageEventHelper;

        mPackageMonitorCallbackHelper = testParams.packageMonitorCallbackHelper;



        registerObservers(false);

        invalidatePackageInfoCache();
@@ -1977,6 +1977,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService 
        mDomainVerificationManager.setConnection(mDomainVerificationConnection);



        mBroadcastHelper = new BroadcastHelper(mInjector);

        mPackageMonitorCallbackHelper = new PackageMonitorCallbackHelper(mInjector);

        mAppDataHelper = new AppDataHelper(this);

        mInstallPackageHelper = new InstallPackageHelper(this, mAppDataHelper);

        mRemovePackageHelper = new RemovePackageHelper(this, mAppDataHelper);

services/core/java/com/android/server/pm/PackageManagerServiceTestParams.java

+1 −0
Original line number Diff line number Diff line
@@ -123,4 +123,5 @@ public final class PackageManagerServiceTestParams { 
    public Set<String> initialNonStoppedSystemPackages = new ArraySet<>();

    public boolean shouldStopSystemPackagesByDefault;

    public FreeStorageHelper freeStorageHelper;

    public PackageMonitorCallbackHelper packageMonitorCallbackHelper;

}

services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java

+10 −2
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import android.content.pm.PackageInstaller; 
import android.content.pm.PackageManager;

import android.net.Uri;

import android.os.Bundle;

import android.os.Handler;

import android.os.IRemoteCallback;

import android.os.RemoteCallbackList;

import android.os.RemoteException;
@@ -45,6 +46,12 @@ class PackageMonitorCallbackHelper { 
    private final Object mLock = new Object();

    final IActivityManager mActivityManager = ActivityManager.getService();



    final Handler mHandler;



    PackageMonitorCallbackHelper(PackageManagerServiceInjector injector) {

        mHandler = injector.getHandler();

    }



    @NonNull

    @GuardedBy("mLock")

    private final RemoteCallbackList<IRemoteCallback> mCallbacks = new RemoteCallbackList<>();
@@ -149,13 +156,14 @@ class PackageMonitorCallbackHelper { 
                intent.putExtra(Intent.EXTRA_UID, uid);

            }

            intent.putExtra(Intent.EXTRA_USER_HANDLE, userId);

            callbacks.broadcast((callback, user) -> {



            mHandler.post(() -> callbacks.broadcast((callback, user) -> {

                int registerUserId = (int) user;

                if ((registerUserId != UserHandle.USER_ALL) && (registerUserId != userId)) {

                    return;

                }

                invokeCallback(callback, intent);

            });

            }));

        }

    }

services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java

+20 −1
Original line number Diff line number Diff line
@@ -24,15 +24,19 @@ import static org.mockito.Mockito.reset; 
import static org.mockito.Mockito.spy;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.content.Intent;

import android.content.pm.PackageInstaller;

import android.content.pm.PackageManager;

import android.os.Bundle;

import android.os.Handler;

import android.os.IRemoteCallback;

import android.os.Looper;



import org.junit.After;

import org.junit.Before;

import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.junit.runners.JUnit4;
@@ -48,11 +52,18 @@ public class PackageMonitorCallbackHelperTest { 


    private static final String FAKE_PACKAGE_NAME = "com.android.server.pm.fakeapp";

    private static final int FAKE_PACKAGE_UID = 123;



    private final Handler mHandler = new Handler(Looper.getMainLooper());

    PackageMonitorCallbackHelper mPackageMonitorCallbackHelper;



    @Rule

    public final MockSystemRule mMockSystem = new MockSystemRule();



    @Before

    public void setup() {

        mPackageMonitorCallbackHelper = new PackageMonitorCallbackHelper();

        when(mMockSystem.mocks().getInjector().getHandler()).thenReturn(mHandler);

        mPackageMonitorCallbackHelper = new PackageMonitorCallbackHelper(

                mMockSystem.mocks().getInjector());

    }
@@ -67,6 +78,7 @@ public class PackageMonitorCallbackHelperTest { 


        mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED,

                FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */);

        Thread.sleep(300);



        verify(callback, never()).sendResult(any());

    }
@@ -78,6 +90,7 @@ public class PackageMonitorCallbackHelperTest { 
        mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */);

        mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED,

                FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0});

        Thread.sleep(300);



        verify(callback, times(1)).sendResult(any());
@@ -85,6 +98,7 @@ public class PackageMonitorCallbackHelperTest { 
        mPackageMonitorCallbackHelper.unregisterPackageMonitorCallback(callback);

        mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED,

                FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */);

        Thread.sleep(300);



        verify(callback, never()).sendResult(any());

    }
@@ -96,6 +110,7 @@ public class PackageMonitorCallbackHelperTest { 
        mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */);

        mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED,

                FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */);

        Thread.sleep(300);



        ArgumentCaptor<Bundle> bundleCaptor = ArgumentCaptor.forClass(Bundle.class);

        verify(callback, times(1)).sendResult(bundleCaptor.capture());
@@ -117,6 +132,7 @@ public class PackageMonitorCallbackHelperTest { 
        // Notify for user 10

        mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED,

                FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{10} /* userIds */);

        Thread.sleep(300);



        verify(callback, never()).sendResult(any());

    }
@@ -132,6 +148,7 @@ public class PackageMonitorCallbackHelperTest { 
        mPackageMonitorCallbackHelper.notifyPackageChanged(FAKE_PACKAGE_NAME,

                false /* dontKillApp */, components, FAKE_PACKAGE_UID, null /* reason */,

                new int[]{0} /* userIds */);

        Thread.sleep(300);



        ArgumentCaptor<Bundle> bundleCaptor = ArgumentCaptor.forClass(Bundle.class);

        verify(callback, times(1)).sendResult(bundleCaptor.capture());
@@ -158,6 +175,7 @@ public class PackageMonitorCallbackHelperTest { 
        mPackageMonitorCallbackHelper.notifyPackageAddedForNewUsers(FAKE_PACKAGE_NAME,

                FAKE_PACKAGE_UID, new int[]{0} /* userIds */, new int[0],

                PackageInstaller.DATA_LOADER_TYPE_STREAMING);

        Thread.sleep(300);



        ArgumentCaptor<Bundle> bundleCaptor = ArgumentCaptor.forClass(Bundle.class);

        verify(callback, times(1)).sendResult(bundleCaptor.capture());
@@ -180,6 +198,7 @@ public class PackageMonitorCallbackHelperTest { 
        mPackageMonitorCallbackHelper.notifyResourcesChanged(true /* mediaStatus */,

                true /* replacing */, new String[]{FAKE_PACKAGE_NAME},

                new int[]{FAKE_PACKAGE_UID} /* uids */);

        Thread.sleep(300);



        ArgumentCaptor<Bundle> bundleCaptor = ArgumentCaptor.forClass(Bundle.class);

        verify(callback, times(1)).sendResult(bundleCaptor.capture());