Resolving race condition while writing recent taskids

There was a race condition, due to which, by the time
TaskPersister started writing the taskids to the file,
the user was stopped and its data from memory was unloaded,
resulting in a NullPointerException

Bug: 30944155
Change-Id: Iac3333b7744241c90a7769686983e3f16e6880c1

services/core/java/com/android/server/am/TaskPersister.java

@@ -87,6 +87,8 @@ public class TaskPersister {
     private final RecentTasks mRecentTasks;
     private final SparseArray<SparseBooleanArray> mTaskIdsInFile = new SparseArray<>();
     private final File mTaskIdsDir;
+    // To lock file operations in TaskPersister
+    private final Object mIoLock = new Object();
 
     /**
      * Value determines write delay mode as follows: < 0 We are Flushing. No delays between writes
@@ -195,52 +197,52 @@ public class TaskPersister {
             return mTaskIdsInFile.get(userId).clone();
         }
         final SparseBooleanArray persistedTaskIds = new SparseBooleanArray();
-        BufferedReader reader = null;
+        synchronized (mIoLock) {
-        String line;
+            BufferedReader reader = null;
-        try {
+            String line;
-            reader = new BufferedReader(new FileReader(getUserPersistedTaskIdsFile(userId)));
+            try {
-            while ((line = reader.readLine()) != null) {
+                reader = new BufferedReader(new FileReader(getUserPersistedTaskIdsFile(userId)));
-                for (String taskIdString : line.split("\\s+")) {
+                while ((line = reader.readLine()) != null) {
-                    int id = Integer.parseInt(taskIdString);
+                    for (String taskIdString : line.split("\\s+")) {
-                    persistedTaskIds.put(id, true);
+                        int id = Integer.parseInt(taskIdString);
+                        persistedTaskIds.put(id, true);
+                    }
                 }
+            } catch (FileNotFoundException e) {
+                // File doesn't exist. Ignore.
+            } catch (Exception e) {
+                Slog.e(TAG, "Error while reading taskIds file for user " + userId, e);
+            } finally {
+                IoUtils.closeQuietly(reader);
             }
-        } catch (FileNotFoundException e) {
-            // File doesn't exist. Ignore.
-        } catch (Exception e) {
-            Slog.e(TAG, "Error while reading taskIds file for user " + userId, e);
-        } finally {
-            IoUtils.closeQuietly(reader);
         }
         mTaskIdsInFile.put(userId, persistedTaskIds);
         return persistedTaskIds.clone();
     }
 
+
     @VisibleForTesting
-    void maybeWritePersistedTaskIdsForUser(@NonNull SparseBooleanArray taskIds, int userId) {
+    void writePersistedTaskIdsForUser(@NonNull SparseBooleanArray taskIds, int userId) {
         if (userId < 0) {
             return;
         }
-        SparseBooleanArray persistedIdsInFile = mTaskIdsInFile.get(userId);
-        if (persistedIdsInFile != null && persistedIdsInFile.equals(taskIds)) {
-            return;
-        }
         final File persistedTaskIdsFile = getUserPersistedTaskIdsFile(userId);
-        BufferedWriter writer = null;
+        synchronized (mIoLock) {
-        try {
+            BufferedWriter writer = null;
-            writer = new BufferedWriter(new FileWriter(persistedTaskIdsFile));
+            try {
-            for (int i = 0; i < taskIds.size(); i++) {
+                writer = new BufferedWriter(new FileWriter(persistedTaskIdsFile));
-                if (taskIds.valueAt(i)) {
+                for (int i = 0; i < taskIds.size(); i++) {
-                    writer.write(String.valueOf(taskIds.keyAt(i)));
+                    if (taskIds.valueAt(i)) {
-                    writer.newLine();
+                        writer.write(String.valueOf(taskIds.keyAt(i)));
+                        writer.newLine();
+                    }
                 }
+            } catch (Exception e) {
+                Slog.e(TAG, "Error while writing taskIds file for user " + userId, e);
+            } finally {
+                IoUtils.closeQuietly(writer);
             }
-        } catch (Exception e) {
-            Slog.e(TAG, "Error while writing taskIds file for user " + userId, e);
-        } finally {
-            IoUtils.closeQuietly(writer);
         }
-        mTaskIdsInFile.put(userId, taskIds.clone());
     }
 
     void unloadUserDataFromMemory(int userId) {
@@ -543,16 +545,23 @@ public class TaskPersister {
     }
 
     private void writeTaskIdsFiles() {
-        int candidateUserIds[];
+        SparseArray<SparseBooleanArray> changedTaskIdsPerUser = new SparseArray<>();
         synchronized (mService) {
-            candidateUserIds = mRecentTasks.usersWithRecentsLoadedLocked();
+            for (int userId : mRecentTasks.usersWithRecentsLoadedLocked()) {
-        }
+                SparseBooleanArray taskIdsToSave = mRecentTasks.mPersistedTaskIds.get(userId);
-        SparseBooleanArray taskIdsToSave;
+                SparseBooleanArray persistedIdsInFile = mTaskIdsInFile.get(userId);
-        for (int userId : candidateUserIds) {
+                if (persistedIdsInFile != null && persistedIdsInFile.equals(taskIdsToSave)) {
-            synchronized (mService) {
+                    continue;
-                taskIdsToSave = mRecentTasks.mPersistedTaskIds.get(userId).clone();
+                } else {
+                    SparseBooleanArray taskIdsToSaveCopy = taskIdsToSave.clone();
+                    mTaskIdsInFile.put(userId, taskIdsToSaveCopy);
+                    changedTaskIdsPerUser.put(userId, taskIdsToSaveCopy);
+                }
             }
-            maybeWritePersistedTaskIdsForUser(taskIdsToSave, userId);
+        }
+        for (int i = 0; i < changedTaskIdsPerUser.size(); i++) {
+            writePersistedTaskIdsForUser(changedTaskIdsPerUser.valueAt(i),
+                    changedTaskIdsPerUser.keyAt(i));
         }
     }
 

services/tests/servicestests/src/com/android/server/am/TaskPersisterTest.java

@@ -62,7 +62,7 @@ public class TaskPersisterTest extends AndroidTestCase {
         for (int i = 0; i < 100; i++) {
             taskIdsOnFile.put(getRandomTaskIdForUser(testUserId), true);
         }
-        mTaskPersister.maybeWritePersistedTaskIdsForUser(taskIdsOnFile, testUserId);
+        mTaskPersister.writePersistedTaskIdsForUser(taskIdsOnFile, testUserId);
         SparseBooleanArray newTaskIdsOnFile = mTaskPersister
                 .loadPersistedTaskIdsForUser(testUserId);
         assertTrue("TaskIds written differ from TaskIds read back from file",