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

Commit f5932e1b authored by Varun Shah's avatar Varun Shah
Browse files

Introduce a finer lock for JobNotificationCoordinator. 

Introduce a new lock for local objects in JobNotificationCoordinator
which are not dependent on other JobScheduler objects. This will allow
for external services like NotificationManagerService to query the
relevant information in JobNotificationCoordinator without JS having to
hold the global lock and causing a deadlock.

Also update NonNull annotations.

Bug: 277694892
Test: atest JobNotificationCoordinatorTest
Test: atest NotificationTest
Test: atest NotificationManagerTest
Change-Id: I17013deb4c71b70a560c8f27e6186396012362e3
parent 99e08bc3

apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java

+2 −2
Original line number Diff line number Diff line
@@ -69,8 +69,8 @@ public interface JobSchedulerInternal { 
     * @return {@code true} if the given notification channel is associated with any user-initiated

     * jobs.

     */

    boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(String notificationChannel,

            int userId, String packageName);

    boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(

            @NonNull String notificationChannel, int userId, @NonNull String packageName);



    /**

     * Report a snapshot of sync-related jobs back to the sync manager

apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java

+3 −5
Original line number Diff line number Diff line
@@ -1925,16 +1925,14 @@ class JobConcurrencyManager { 
        return null;

    }



    @GuardedBy("mLock")

    boolean isNotificationAssociatedWithAnyUserInitiatedJobs(int notificationId, int userId,

            String packageName) {

            @NonNull String packageName) {

        return mNotificationCoordinator.isNotificationAssociatedWithAnyUserInitiatedJobs(

                notificationId, userId, packageName);

    }



    @GuardedBy("mLock")

    boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(String notificationChannel,

            int userId, String packageName) {

    boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(

            @NonNull String notificationChannel, int userId, @NonNull String packageName) {

        return mNotificationCoordinator.isNotificationChannelAssociatedWithAnyUserInitiatedJobs(

                notificationChannel, userId, packageName);

    }

apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java

+122 −15
Original line number Diff line number Diff line
@@ -27,9 +27,12 @@ import android.content.pm.UserPackage; 
import android.os.UserHandle;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.IntArray;

import android.util.Slog;

import android.util.SparseArrayMap;

import android.util.SparseSetArray;



import com.android.internal.annotations.GuardedBy;

import com.android.server.LocalServices;

import com.android.server.job.controllers.JobStatus;

import com.android.server.notification.NotificationManagerInternal;
@@ -37,6 +40,14 @@ import com.android.server.notification.NotificationManagerInternal; 
class JobNotificationCoordinator {

    private static final String TAG = "JobNotificationCoordinator";



    /**

     * Local lock for independent objects like mUijNotifications and mUijNotificationChannels which

     * don't depend on other JS objects such as JobServiceContext which require the global JS lock.

     *

     * Note: do <b>NOT</b> acquire the global lock while this one is held.

     */

    private final Object mUijLock = new Object();



    /**

     * Mapping of UserPackage -> {notificationId -> List<JobServiceContext>} to track which jobs

     * are associated with each app's notifications.
@@ -49,6 +60,27 @@ class JobNotificationCoordinator { 
    private final ArrayMap<JobServiceContext, NotificationDetails> mNotificationDetails =

            new ArrayMap<>();



    /**

     * Mapping of userId -> {packageName, notificationIds} tracking which notifications

     * associated with each app belong to user-initiated jobs.

     *

     * Note: this map can be accessed without holding the main JS lock, so that other services like

     * NotificationManagerService can call into JS and verify associations.

     */

    @GuardedBy("mUijLock")

    private final SparseArrayMap<String, IntArray> mUijNotifications = new SparseArrayMap<>();



    /**

     * Mapping of userId -> {packageName, notificationChannels} tracking which notification channels

     * associated with each app are hosting a user-initiated job notification.

     *

     * Note: this map can be accessed without holding the main JS lock, so that other services like

     * NotificationManagerService can call into JS and verify associations.

     */

    @GuardedBy("mUijLock")

    private final SparseArrayMap<String, ArraySet<String>> mUijNotificationChannels =

            new SparseArrayMap<>();



    private static final class NotificationDetails {

        @NonNull

        public final UserPackage userPackage;
@@ -81,15 +113,24 @@ class JobNotificationCoordinator { 
            int callingPid, int callingUid, int notificationId, @NonNull Notification notification,

            @JobService.JobEndNotificationPolicy int jobEndNotificationPolicy) {

        validateNotification(packageName, callingUid, notification, jobEndNotificationPolicy);

        final JobStatus jobStatus = hostingContext.getRunningJobLocked();

        final NotificationDetails oldDetails = mNotificationDetails.get(hostingContext);

        if (oldDetails != null && oldDetails.notificationId != notificationId) {

            // App is switching notification IDs. Remove association with the old one.

            removeNotificationAssociation(hostingContext, JobParameters.STOP_REASON_UNDEFINED);

            removeNotificationAssociation(hostingContext, JobParameters.STOP_REASON_UNDEFINED,

                    jobStatus);

        }

        final int userId = UserHandle.getUserId(callingUid);

        final JobStatus jobStatus = hostingContext.getRunningJobLocked();

        if (jobStatus != null && jobStatus.startedAsUserInitiatedJob) {

            notification.flags |= Notification.FLAG_USER_INITIATED_JOB;

            synchronized (mUijLock) {

                maybeCreateUijNotificationSetsLocked(userId, packageName);

                final IntArray notificationIds = mUijNotifications.get(userId, packageName);

                if (notificationIds.indexOf(notificationId) == -1) {

                    notificationIds.add(notificationId);

                }

                mUijNotificationChannels.get(userId, packageName).add(notification.getChannelId());

            }

        }

        final UserPackage userPackage = UserPackage.of(userId, packageName);

        final NotificationDetails details = new NotificationDetails(
@@ -110,7 +151,7 @@ class JobNotificationCoordinator { 
    }



    void removeNotificationAssociation(@NonNull JobServiceContext hostingContext,

            @JobParameters.StopReason int stopReason) {

            @JobParameters.StopReason int stopReason, JobStatus completedJob) {

        final NotificationDetails details = mNotificationDetails.remove(hostingContext);

        if (details == null) {

            return;
@@ -121,10 +162,11 @@ class JobNotificationCoordinator { 
            Slog.wtf(TAG, "Association data structures not in sync");

            return;

        }

        final String packageName = details.userPackage.packageName;

        final int userId = UserHandle.getUserId(details.appUid);

        final String packageName = details.userPackage.packageName;

        final int notificationId = details.notificationId;

        boolean stripUijFlag = true;

        ArraySet<JobServiceContext> associatedContexts = associations.get(details.notificationId);

        ArraySet<JobServiceContext> associatedContexts = associations.get(notificationId);

        if (associatedContexts == null || associatedContexts.isEmpty()) {

            // No more jobs using this notification. Apply the final job stop policy.

            // If the user attempted to stop the job/app, then always remove the notification
@@ -133,23 +175,50 @@ class JobNotificationCoordinator { 
                    || stopReason == JobParameters.STOP_REASON_USER) {

                mNotificationManagerInternal.cancelNotification(

                        packageName, packageName, details.appUid, details.appPid, /* tag */ null,

                        details.notificationId, userId);

                        notificationId, userId);

                stripUijFlag = false;

            }

        } else {

            // Strip the UIJ flag only if there are no other UIJs associated with the notification

            stripUijFlag = !isNotificationAssociatedWithAnyUserInitiatedJobs(

                    details.notificationId, userId, packageName);

            stripUijFlag = !isNotificationUsedForAnyUij(userId, packageName, notificationId);

        }

        if (stripUijFlag) {

            // Strip the user-initiated job flag from the notification.

            mNotificationManagerInternal.removeUserInitiatedJobFlagFromNotification(

                    packageName, details.notificationId, userId);

                    packageName, notificationId, userId);

        }



        // Clean up UIJ related objects if the just completed job was a UIJ

        if (completedJob != null && completedJob.startedAsUserInitiatedJob) {

            maybeDeleteNotificationIdAssociation(userId, packageName, notificationId);

            maybeDeleteNotificationChannelAssociation(

                    userId, packageName, details.notificationChannel);

        }

    }



    boolean isNotificationAssociatedWithAnyUserInitiatedJobs(int notificationId,

            int userId, String packageName) {

            int userId, @NonNull String packageName) {

        synchronized (mUijLock) {

            final IntArray notifications = mUijNotifications.get(userId, packageName);

            if (notifications != null) {

                return notifications.indexOf(notificationId) != -1;

            }

            return false;

        }

    }



    boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(

            @NonNull String notificationChannel, int userId, @NonNull String packageName) {

        synchronized (mUijLock) {

            final ArraySet<String> channels = mUijNotificationChannels.get(userId, packageName);

            if (channels != null) {

                return channels.contains(notificationChannel);

            }

            return false;

        }

    }



    private boolean isNotificationUsedForAnyUij(int userId, String packageName,

            int notificationId) {

        final UserPackage pkgDetails = UserPackage.of(userId, packageName);

        final SparseSetArray<JobServiceContext> associations = mCurrentAssociations.get(pkgDetails);

        if (associations == null) {
@@ -170,8 +239,26 @@ class JobNotificationCoordinator { 
        return false;

    }



    boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(String notificationChannel,

            int userId, String packageName) {

    private void maybeDeleteNotificationIdAssociation(int userId, String packageName,

            int notificationId) {

        if (isNotificationUsedForAnyUij(userId, packageName, notificationId)) {

            return;

        }



        // Safe to delete - no UIJs for this package are using this notification id

        synchronized (mUijLock) {

            final IntArray notifications = mUijNotifications.get(userId, packageName);

            if (notifications != null) {

                notifications.remove(notifications.indexOf(notificationId));

                if (notifications.size() == 0) {

                    mUijNotifications.delete(userId, packageName);

                }

            }

        }

    }



    private void maybeDeleteNotificationChannelAssociation(int userId, String packageName,

            String notificationChannel) {

        for (int i = mNotificationDetails.size() - 1; i >= 0; i--) {

            final JobServiceContext jsc = mNotificationDetails.keyAt(i);

            final NotificationDetails details = mNotificationDetails.get(jsc);
@@ -183,11 +270,31 @@ class JobNotificationCoordinator { 
                    && details.notificationChannel.equals(notificationChannel)) {

                final JobStatus jobStatus = jsc.getRunningJobLocked();

                if (jobStatus != null && jobStatus.startedAsUserInitiatedJob) {

                    return true;

                    return;

                }

            }

        }

        return false;



        // Safe to delete - no UIJs for this package are using this notification channel

        synchronized (mUijLock) {

            ArraySet<String> channels = mUijNotificationChannels.get(userId, packageName);

            if (channels != null) {

                channels.remove(notificationChannel);

                if (channels.isEmpty()) {

                    mUijNotificationChannels.delete(userId, packageName);

                }

            }

        }

    }



    @GuardedBy("mUijLock")

    private void maybeCreateUijNotificationSetsLocked(int userId, String packageName) {

        if (mUijNotifications.get(userId, packageName) == null) {

            mUijNotifications.add(userId, packageName, new IntArray());

        }

        if (mUijNotificationChannels.get(userId, packageName) == null) {

            mUijNotificationChannels.add(userId, packageName, new ArraySet<>());

        }

    }



    private void validateNotification(@NonNull String packageName, int callingUid,

apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java

+6 −10
Original line number Diff line number Diff line
@@ -3713,27 +3713,23 @@ public class JobSchedulerService extends com.android.server.SystemService 


        @Override

        public boolean isNotificationAssociatedWithAnyUserInitiatedJobs(int notificationId,

                int userId, String packageName) {

                int userId, @NonNull String packageName) {

            if (packageName == null) {

                return false;

            }

            synchronized (mLock) {

            return mConcurrencyManager.isNotificationAssociatedWithAnyUserInitiatedJobs(

                    notificationId, userId, packageName);

        }

        }



        @Override

        public boolean isNotificationChannelAssociatedWithAnyUserInitiatedJobs(

                String notificationChannel, int userId, String packageName) {

                @NonNull String notificationChannel, int userId, @NonNull String packageName) {

            if (packageName == null || notificationChannel == null) {

                return false;

            }

            synchronized (mLock) {

            return mConcurrencyManager.isNotificationChannelAssociatedWithAnyUserInitiatedJobs(

                    notificationChannel, userId, packageName);

        }

        }



        @Override

        public JobStorePersistStats getPersistStats() {

apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java

+2 −1
Original line number Diff line number Diff line
@@ -1464,7 +1464,8 @@ public final class JobServiceContext implements ServiceConnection { 
                    JobSchedulerEconomicPolicy.ACTION_JOB_TIMEOUT,

                    String.valueOf(mRunningJob.getJobId()));

        }

        mNotificationCoordinator.removeNotificationAssociation(this, reschedulingStopReason);

        mNotificationCoordinator.removeNotificationAssociation(this,

                reschedulingStopReason, completedJob);

        if (mWakeLock != null) {

            mWakeLock.release();

        }