Stop restoring tasks added before recent is loaded
Previously we can have duplicated tasks sharing the same taskId in
RecentTasks due to race condition.
1. A task is created before RecentTasks#loadUserRecentsLocked
(e.g. through adb)
2. RecentTasks#notifyTaskPersisterLocked eventually writes the task file
to storage (e.g. XX_task.xml)
3. RecentTasks#loadUserRecentsLocked tries to recover XX_task.xml while
the task has already been added to RecentTasks.
To fix the issue, the CL stops restoring tasks added before recent is
loaded.
Bug: 36796576
Test: Build and boot Android, check the recent is correctly loaded
Change-Id: Ib57977f2a0a63f7bf7db4d3fd70bdcc359e76f7d
(cherry picked from commit b8aeb6f12e51e6d3ecf6a5f40c953dc76ff64884)
diff --git a/services/core/java/com/android/server/am/RecentTasks.java b/services/core/java/com/android/server/am/RecentTasks.java
index 6a13d36..a6ebac4 100644
--- a/services/core/java/com/android/server/am/RecentTasks.java
+++ b/services/core/java/com/android/server/am/RecentTasks.java
@@ -41,6 +41,7 @@
import android.os.Environment;
import android.os.RemoteException;
import android.os.UserHandle;
+import android.util.ArraySet;
import android.util.Slog;
import android.util.SparseArray;
import android.util.SparseBooleanArray;
@@ -104,13 +105,29 @@
* @param userId the user Id
*/
void loadUserRecentsLocked(int userId) {
- if (!mUsersWithRecentsLoaded.get(userId)) {
- // Load the task ids if not loaded.
- loadPersistedTaskIdsForUserLocked(userId);
- Slog.i(TAG, "Loading recents for user " + userId + " into memory.");
- addAll(mTaskPersister.restoreTasksForUserLocked(userId));
- cleanupLocked(userId);
- mUsersWithRecentsLoaded.put(userId, true);
+ if (mUsersWithRecentsLoaded.get(userId)) {
+ return;
+ }
+
+ // Load the task ids if not loaded.
+ loadPersistedTaskIdsForUserLocked(userId);
+
+ // Check if any tasks are added before recents is loaded
+ final SparseBooleanArray preaddedTasks = new SparseBooleanArray();
+ for (final TaskRecord task : this) {
+ if (task.userId == userId && shouldPersistTaskLocked(task)) {
+ preaddedTasks.put(task.taskId, true);
+ }
+ }
+
+ Slog.i(TAG, "Loading recents for user " + userId + " into memory.");
+ addAll(mTaskPersister.restoreTasksForUserLocked(userId, preaddedTasks));
+ cleanupLocked(userId);
+ mUsersWithRecentsLoaded.put(userId, true);
+
+ // If we have tasks added before loading recents, we need to update persistent task IDs.
+ if (preaddedTasks.size() > 0) {
+ syncPersistentTaskIdsLocked();
}
}
@@ -149,8 +166,7 @@
}
for (int i = size() - 1; i >= 0; i--) {
final TaskRecord task = get(i);
- final ActivityStack stack = task.getStack();
- if (task.isPersistable && (stack == null || !stack.isHomeOrRecentsStack())) {
+ if (shouldPersistTaskLocked(task)) {
// Set of persisted taskIds for task.userId should not be null here
// TODO Investigate why it can happen. For now initialize with an empty set
if (mPersistedTaskIds.get(task.userId) == null) {
@@ -163,6 +179,10 @@
}
}
+ private static boolean shouldPersistTaskLocked(TaskRecord task) {
+ final ActivityStack<?> stack = task.getStack();
+ return task.isPersistable && (stack == null || !stack.isHomeOrRecentsStack());
+ }
void onSystemReadyLocked() {
clear();
diff --git a/services/core/java/com/android/server/am/TaskPersister.java b/services/core/java/com/android/server/am/TaskPersister.java
index 9deabb3..e56b09d 100644
--- a/services/core/java/com/android/server/am/TaskPersister.java
+++ b/services/core/java/com/android/server/am/TaskPersister.java
@@ -78,9 +78,8 @@
/** Special value for mWriteTime to mean don't wait, just write */
private static final long FLUSH_QUEUE = -1;
- private static final String RECENTS_FILENAME = "_task";
private static final String TASKS_DIRNAME = "recent_tasks";
- private static final String TASK_EXTENSION = ".xml";
+ private static final String TASK_FILENAME_SUFFIX = "_task.xml";
private static final String IMAGES_DIRNAME = "recent_images";
private static final String PERSISTED_TASK_IDS_FILENAME = "persisted_taskIds.txt";
static final String IMAGE_EXTENSION = ".png";
@@ -412,7 +411,7 @@
return null;
}
- List<TaskRecord> restoreTasksForUserLocked(final int userId) {
+ List<TaskRecord> restoreTasksForUserLocked(final int userId, SparseBooleanArray preaddedTasks) {
final ArrayList<TaskRecord> tasks = new ArrayList<TaskRecord>();
ArraySet<Integer> recoveredTaskIds = new ArraySet<Integer>();
@@ -430,6 +429,24 @@
Slog.d(TAG, "restoreTasksForUserLocked: userId=" + userId
+ ", taskFile=" + taskFile.getName());
}
+
+ if (!taskFile.getName().endsWith(TASK_FILENAME_SUFFIX)) {
+ continue;
+ }
+ try {
+ final int taskId = Integer.parseInt(taskFile.getName().substring(
+ 0 /* beginIndex */,
+ taskFile.getName().length() - TASK_FILENAME_SUFFIX.length()));
+ if (preaddedTasks.get(taskId, false)) {
+ Slog.w(TAG, "Task #" + taskId +
+ " has already been created so we don't restore again");
+ continue;
+ }
+ } catch (NumberFormatException e) {
+ Slog.w(TAG, "Unexpected task file name", e);
+ continue;
+ }
+
BufferedReader reader = null;
boolean deleteFile = false;
try {
@@ -744,13 +761,11 @@
try {
atomicFile = new AtomicFile(new File(
getUserTasksDir(task.userId),
- String.valueOf(task.taskId) + RECENTS_FILENAME
- + TASK_EXTENSION));
+ String.valueOf(task.taskId) + TASK_FILENAME_SUFFIX));
file = atomicFile.startWrite();
file.write(stringWriter.toString().getBytes());
file.write('\n');
atomicFile.finishWrite(file);
-
} catch (IOException e) {
if (file != null) {
atomicFile.failWrite(file);