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

Commit b8aeb6f1 authored by Daichi Hirono's avatar Daichi Hirono
Browse files

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
parent 1ce83f00

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

+29 −9
Original line number Original line Diff line number Diff line
@@ -41,6 +41,7 @@ import android.graphics.Bitmap; 
import android.os.Environment;

import android.os.Environment;

import android.os.RemoteException;

import android.os.RemoteException;

import android.os.UserHandle;

import android.os.UserHandle;

import android.util.ArraySet;

import android.util.Slog;

import android.util.Slog;

import android.util.SparseArray;

import android.util.SparseArray;

import android.util.SparseBooleanArray;

import android.util.SparseBooleanArray;
@@ -104,13 +105,29 @@ class RecentTasks extends ArrayList<TaskRecord> { 
     * @param userId the user Id

     * @param userId the user Id

     */

     */

    void loadUserRecentsLocked(int userId) {

    void loadUserRecentsLocked(int userId) {

        if (!mUsersWithRecentsLoaded.get(userId)) {

        if (mUsersWithRecentsLoaded.get(userId)) {

            return;

        }



        // Load the task ids if not loaded.

        // Load the task ids if not loaded.

        loadPersistedTaskIdsForUserLocked(userId);

        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.");

        Slog.i(TAG, "Loading recents for user " + userId + " into memory.");

            addAll(mTaskPersister.restoreTasksForUserLocked(userId));

        addAll(mTaskPersister.restoreTasksForUserLocked(userId, preaddedTasks));

        cleanupLocked(userId);

        cleanupLocked(userId);

        mUsersWithRecentsLoaded.put(userId, true);

        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 @@ class RecentTasks extends ArrayList<TaskRecord> { 
        }

        }

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

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

            final TaskRecord task = get(i);

            final TaskRecord task = get(i);

            final ActivityStack stack = task.getStack();

            if (shouldPersistTaskLocked(task)) {

            if (task.isPersistable && (stack == null || !stack.isHomeOrRecentsStack())) {

                // Set of persisted taskIds for task.userId should not be null here

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

                // TODO Investigate why it can happen. For now initialize with an empty set

                if (mPersistedTaskIds.get(task.userId) == null) {

                if (mPersistedTaskIds.get(task.userId) == null) {
@@ -163,6 +179,10 @@ class RecentTasks extends ArrayList<TaskRecord> { 
        }

        }

    }

    }





    private static boolean shouldPersistTaskLocked(TaskRecord task) {

        final ActivityStack<?> stack = task.getStack();

        return task.isPersistable && (stack == null || !stack.isHomeOrRecentsStack());

    }





    void onSystemReadyLocked() {

    void onSystemReadyLocked() {

        clear();

        clear();

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

+19 −6
Original line number Original line Diff line number Diff line
@@ -78,9 +78,8 @@ public class TaskPersister { 
    /** Special value for mWriteTime to mean don't wait, just write */

    /** Special value for mWriteTime to mean don't wait, just write */

    private static final long FLUSH_QUEUE = -1;

    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 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 IMAGES_DIRNAME = "recent_images";

    private static final String PERSISTED_TASK_IDS_FILENAME = "persisted_taskIds.txt";

    private static final String PERSISTED_TASK_IDS_FILENAME = "persisted_taskIds.txt";

    static final String IMAGE_EXTENSION = ".png";

    static final String IMAGE_EXTENSION = ".png";
@@ -412,7 +411,7 @@ public class TaskPersister { 
        return null;

        return null;

    }

    }





    List<TaskRecord> restoreTasksForUserLocked(final int userId) {

    List<TaskRecord> restoreTasksForUserLocked(final int userId, SparseBooleanArray preaddedTasks) {

        final ArrayList<TaskRecord> tasks = new ArrayList<TaskRecord>();

        final ArrayList<TaskRecord> tasks = new ArrayList<TaskRecord>();

        ArraySet<Integer> recoveredTaskIds = new ArraySet<Integer>();

        ArraySet<Integer> recoveredTaskIds = new ArraySet<Integer>();
@@ -430,6 +429,22 @@ public class TaskPersister { 
                Slog.d(TAG, "restoreTasksForUserLocked: userId=" + userId

                Slog.d(TAG, "restoreTasksForUserLocked: userId=" + userId

                        + ", taskFile=" + taskFile.getName());

                        + ", taskFile=" + taskFile.getName());

            }

            }



            if (!taskFile.getName().endsWith(TASK_FILENAME_SUFFIX)) {

                continue;

            }

            try {

                final int taskId = Integer.parseInt(taskFile.getName().substring(

                        0, 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) {

                continue;

            }



            BufferedReader reader = null;

            BufferedReader reader = null;

            boolean deleteFile = false;

            boolean deleteFile = false;

            try {

            try {
@@ -744,13 +759,11 @@ public class TaskPersister { 
                        try {

                        try {

                            atomicFile = new AtomicFile(new File(

                            atomicFile = new AtomicFile(new File(

                                    getUserTasksDir(task.userId),

                                    getUserTasksDir(task.userId),

                                    String.valueOf(task.taskId) + RECENTS_FILENAME

                                    String.valueOf(task.taskId) + TASK_FILENAME_SUFFIX));

                                    + TASK_EXTENSION));

                            file = atomicFile.startWrite();

                            file = atomicFile.startWrite();

                            file.write(stringWriter.toString().getBytes());

                            file.write(stringWriter.toString().getBytes());

                            file.write('\n');

                            file.write('\n');

                            atomicFile.finishWrite(file);

                            atomicFile.finishWrite(file);



                        } catch (IOException e) {

                        } catch (IOException e) {

                            if (file != null) {

                            if (file != null) {

                                atomicFile.failWrite(file);

                                atomicFile.failWrite(file);