Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Introduce a finer lock for JobNotificationCoordinator." into udc-dev

Browse Source
This commit is contained in:
Varun Shah
2023-04-13 03:49:25 +00:00
committed by Android (Google) Code Review
parent 778e47bb49 f5932e1b4a
commit f1eb79280b
6 changed files with 161 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
apex/jobscheduler/framework/java/com/android/server/job/JobSchedulerInternal.java
View File

@@ -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

8
apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java
View File

@@ -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);
}

137
apex/jobscheduler/service/java/com/android/server/job/JobNotificationCoordinator.java
View File

@@ -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,

16
apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
View File

@@ -3713,26 +3713,22 @@ 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);
}
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);
}
return mConcurrencyManager.isNotificationChannelAssociatedWithAnyUserInitiatedJobs(
notificationChannel, userId, packageName);
}
@Override

3
apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java
View File

@@ -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();
}

39
services/tests/mockingservicestests/src/com/android/server/job/JobNotificationCoordinatorTest.java
View File

@@ -149,7 +149,8 @@ public class JobNotificationCoordinatorTest {
.enqueueNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId), eq(notification), eq(UserHandle.getUserId(uid)));
coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED,
jsc.getRunningJobLocked());
verify(mNotificationManagerInternal, never())
.cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(),
anyInt(), anyInt());
@@ -170,7 +171,8 @@ public class JobNotificationCoordinatorTest {
.enqueueNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId), eq(notification), eq(UserHandle.getUserId(uid)));
coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_UNDEFINED,
jsc.getRunningJobLocked());
verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid)));
@@ -292,7 +294,8 @@ public class JobNotificationCoordinatorTest {
eq(notificationId2), eq(notification2), eq(UserHandle.getUserId(uid)));
// Remove the first job. Only the first notification should be removed.
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED,
jsc1.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId1), eq(UserHandle.getUserId(uid)));
@@ -300,7 +303,8 @@ public class JobNotificationCoordinatorTest {
.cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(),
eq(notificationId2), anyInt());
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED,
jsc2.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId2), eq(UserHandle.getUserId(uid)));
@@ -335,12 +339,14 @@ public class JobNotificationCoordinatorTest {
eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid)));
// Remove the first job. The notification shouldn't be touched because of the 2nd job.
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED,
jsc1.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal, never())
.cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(),
anyInt(), anyInt());
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED,
jsc2.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid)));
@@ -376,7 +382,8 @@ public class JobNotificationCoordinatorTest {
eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid2)));
// Remove the first job. Only the first notification should be removed.
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED,
jsc1.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid1), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid1)));
@@ -384,7 +391,8 @@ public class JobNotificationCoordinatorTest {
.cancelNotification(anyString(), anyString(), eq(uid2), anyInt(), any(),
anyInt(), anyInt());
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED,
jsc2.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid2), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid2)));
@@ -421,7 +429,8 @@ public class JobNotificationCoordinatorTest {
eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid)));
// Remove the first job. Only the first notification should be removed.
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_UNDEFINED,
jsc1.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(pkg1), eq(pkg1), eq(uid), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid)));
@@ -429,7 +438,8 @@ public class JobNotificationCoordinatorTest {
.cancelNotification(anyString(), anyString(), eq(uid), anyInt(), any(),
anyInt(), anyInt());
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED);
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_UNDEFINED,
jsc2.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(pkg2), eq(pkg2), eq(uid), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid)));
@@ -450,7 +460,8 @@ public class JobNotificationCoordinatorTest {
.enqueueNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId), eq(notification), eq(UserHandle.getUserId(uid)));
coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_USER);
coordinator.removeNotificationAssociation(jsc, JobParameters.STOP_REASON_USER,
jsc.getRunningJobLocked());
verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid)));
@@ -485,12 +496,14 @@ public class JobNotificationCoordinatorTest {
eq(notificationId), eq(notification2), eq(UserHandle.getUserId(uid)));
// Remove the first job. The notification shouldn't be touched because of the 2nd job.
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_USER);
coordinator.removeNotificationAssociation(jsc1, JobParameters.STOP_REASON_USER,
jsc1.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal, never())
.cancelNotification(anyString(), anyString(), anyInt(), anyInt(), any(),
anyInt(), anyInt());
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_USER);
coordinator.removeNotificationAssociation(jsc2, JobParameters.STOP_REASON_USER,
jsc2.getRunningJobLocked());
inOrder.verify(mNotificationManagerInternal)
.cancelNotification(eq(TEST_PACKAGE), eq(TEST_PACKAGE), eq(uid), eq(pid), any(),
eq(notificationId), eq(UserHandle.getUserId(uid)));