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

Commit f4f8bb79 authored by Craig Mautner's avatar Craig Mautner
Browse files

Eliminate memory leak in TaskPersister

Bitmaps added to TaskPersister were piling up in the queue.

- The mRecentsChanged flag was being modified without holding the
lock. There is no mRecentsChanged flag now. Everything to be
written goes into a queue.
- TaskPersister now runs until the queue is empty.
- Bitmaps being written to the same file were being added to the
end of the queue without replacing the earlier bitmap. Now we
search the queue for matching tasks and replace the bitmaps
if needed.
- Method notify() was renamed to wakeup() so IDE could find usages
quicker.
- Bitmaps that were being requested but were still in the queue
are now being fetched from the queue.

Fixes bug 16512870.

Change-Id: Idca1c712e5d2df8196e93faaf563a54405ee96bf
parent f52773fd
Loading
Loading
Loading
Loading
+7 −3
Original line number Diff line number Diff line
@@ -3698,7 +3698,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                // specified, then replace it with the existing recent task.
                task = tr;
            }
            mTaskPersister.notify(tr, false);
            notifyTaskPersisterLocked(tr, false);
        }
        if (N >= MAX_RECENT_TASKS) {
            final TaskRecord tr = mRecentTasks.remove(N - 1);
@@ -7646,7 +7646,7 @@ public final class ActivityManagerService extends ActivityManagerNative
            tr.removeTaskActivitiesLocked();
            cleanUpRemovedTaskLocked(tr, flags);
            if (tr.isPersistable) {
                notifyTaskPersisterLocked(tr, true);
                notifyTaskPersisterLocked(null, true);
            }
            return true;
        }
@@ -9089,7 +9089,11 @@ public final class ActivityManagerService extends ActivityManagerNative
    }
    void notifyTaskPersisterLocked(TaskRecord task, boolean flush) {
        mTaskPersister.notify(task, flush);
        if (task != null && task.stack != null && task.stack.isHomeStack()) {
            // Never persist the home stack.
            return;
        }
        mTaskPersister.wakeup(task, flush);
    }
    @Override
+190 −118
Original line number Diff line number Diff line
@@ -39,15 +39,17 @@ import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.LinkedList;

public class TaskPersister {
    static final String TAG = "TaskPersister";
    static final boolean DEBUG = false;

    /** When in slow mode don't write tasks out faster than this */
    private static final long INTER_TASK_DELAY_MS = 10000;
    private static final long DEBUG_INTER_TASK_DELAY_MS = 5000;
    /** When not flushing don't write out files faster than this */
    private static final long INTER_WRITE_DELAY_MS = 500;

    /** When not flushing delay this long before writing the first file out. This gives the next
     * task being launched a chance to load its resources without this occupying IO bandwidth. */
    private static final long PRE_TASK_DELAY_MS = 3000;

    private static final String RECENTS_FILENAME = "_task";
    private static final String TASKS_DIRNAME = "recent_tasks";
@@ -63,10 +65,34 @@ public class TaskPersister {
    private final ActivityManagerService mService;
    private final ActivityStackSupervisor mStackSupervisor;

    private boolean mRecentsChanged = false;
    /** Value determines write delay mode as follows:
     *    < 0 We are Flushing. No delays between writes until the image queue is drained and all
     * tasks needing persisting are written to disk. There is no delay between writes.
     *    == 0 We are Idle. Next writes will be delayed by #PRE_TASK_DELAY_MS.
     *    > 0 We are Actively writing. Next write will be at this time. Subsequent writes will be
     * delayed by #INTER_WRITE_DELAY_MS. */
    private long mNextWriteTime = 0;

    private final LazyTaskWriterThread mLazyTaskWriterThread;

    private static class WriteQueueItem {}
    private static class TaskWriteQueueItem extends WriteQueueItem {
        final TaskRecord mTask;
        TaskWriteQueueItem(TaskRecord task) {
            mTask = task;
        }
    }
    private static class ImageWriteQueueItem extends WriteQueueItem {
        final String mFilename;
        Bitmap mImage;
        ImageWriteQueueItem(String filename, Bitmap image) {
            mFilename = filename;
            mImage = image;
        }
    }

    ArrayList<WriteQueueItem> mWriteQueue = new ArrayList<WriteQueueItem>();

    TaskPersister(File systemDir, ActivityStackSupervisor stackSupervisor) {
        sTasksDir = new File(systemDir, TASKS_DIRNAME);
        if (!sTasksDir.exists()) {
@@ -94,19 +120,77 @@ public class TaskPersister {
        mLazyTaskWriterThread.start();
    }

    public void notify(TaskRecord task, boolean flush) {
        if (DEBUG) Slog.d(TAG, "notify: task=" + task + " flush=" + flush +
                " Callers=" + Debug.getCallers(4));
    void wakeup(TaskRecord task, boolean flush) {
        synchronized (this) {
            if (task != null) {
            task.needsPersisting = true;
                int queueNdx;
                for (queueNdx = mWriteQueue.size() - 1; queueNdx >= 0; --queueNdx) {
                    final WriteQueueItem item = mWriteQueue.get(queueNdx);
                    if (item instanceof TaskWriteQueueItem &&
                            ((TaskWriteQueueItem) item).mTask == task) {
                        break;
                    }
                }
                if (queueNdx < 0) {
                    mWriteQueue.add(new TaskWriteQueueItem(task));
                }
            } else {
                // Dummy.
                mWriteQueue.add(new WriteQueueItem());
            }
            if (flush) {
                mNextWriteTime = -1;
            } else if (mNextWriteTime == 0) {
                mNextWriteTime = SystemClock.uptimeMillis() + PRE_TASK_DELAY_MS;
            }
            if (DEBUG) Slog.d(TAG, "wakeup: task=" + task + " flush=" + flush + " mNextWriteTime="
                    + mNextWriteTime + " Callers=" + Debug.getCallers(4));
            notifyAll();
        }
    }

    void saveImage(Bitmap image, String filename) {
        synchronized (this) {
            mLazyTaskWriterThread.mSlow = !flush;
            mRecentsChanged = true;
            int queueNdx;
            for (queueNdx = mWriteQueue.size() - 1; queueNdx >= 0; --queueNdx) {
                final WriteQueueItem item = mWriteQueue.get(queueNdx);
                if (item instanceof ImageWriteQueueItem) {
                    ImageWriteQueueItem imageWriteQueueItem = (ImageWriteQueueItem) item;
                    if (imageWriteQueueItem.mFilename.equals(filename)) {
                        // replace the Bitmap with the new one.
                        imageWriteQueueItem.mImage = image;
                        break;
                    }
                }
            }
            if (queueNdx < 0) {
                mWriteQueue.add(new ImageWriteQueueItem(filename, image));
            }
            if (mNextWriteTime == 0) {
                mNextWriteTime = SystemClock.uptimeMillis() + PRE_TASK_DELAY_MS;
            }
            if (DEBUG) Slog.d(TAG, "saveImage: filename=" + filename + " now=" +
                    SystemClock.uptimeMillis() + " mNextWriteTime=" +
                    mNextWriteTime + " Callers=" + Debug.getCallers(4));
            notifyAll();
        }
    }

    Bitmap getThumbnail(String filename) {
        synchronized (this) {
            for (int queueNdx = mWriteQueue.size() - 1; queueNdx >= 0; --queueNdx) {
                final WriteQueueItem item = mWriteQueue.get(queueNdx);
                if (item instanceof ImageWriteQueueItem) {
                    ImageWriteQueueItem imageWriteQueueItem = (ImageWriteQueueItem) item;
                    if (imageWriteQueueItem.mFilename.equals(filename)) {
                        return imageWriteQueueItem.mImage;
                    }
                }
            }
            return null;
        }
    }

    private StringWriter saveToXml(TaskRecord task) throws IOException, XmlPullParserException {
        if (DEBUG) Slog.d(TAG, "saveToXml: task=" + task);
        final XmlSerializer xmlSerializer = new FastXmlSerializer();
@@ -129,10 +213,6 @@ public class TaskPersister {
        return stringWriter;
    }

    void saveImage(Bitmap image, String filename) {
        mLazyTaskWriterThread.saveImage(image, filename);
    }

    private String fileToString(File file) {
        final String newline = System.lineSeparator();
        try {
@@ -197,7 +277,7 @@ public class TaskPersister {
                                    task);
                            if (task != null) {
                                task.isPersistable = true;
                                task.needsPersisting = true;
                                mWriteQueue.add(new TaskWriteQueueItem(task));
                                tasks.add(task);
                                final int taskId = task.taskId;
                                recoveredTaskIds.add(taskId);
@@ -262,6 +342,8 @@ public class TaskPersister {
    }

    private static void removeObsoleteFiles(ArraySet<Integer> persistentTaskIds, File[] files) {
        if (DEBUG) Slog.d(TAG, "removeObsoleteFile: persistentTaskIds=" + persistentTaskIds +
                " files=" + files);
        for (int fileNdx = 0; fileNdx < files.length; ++fileNdx) {
            File file = files[fileNdx];
            String filename = file.getName();
@@ -270,6 +352,7 @@ public class TaskPersister {
                final int taskId;
                try {
                    taskId = Integer.valueOf(filename.substring(0, taskIdEnd));
                    if (DEBUG) Slog.d(TAG, "removeObsoleteFile: Found taskId=" + taskId);
                } catch (Exception e) {
                    Slog.wtf(TAG, "removeObsoleteFile: Can't parse file=" + file.getName());
                    file.delete();
@@ -295,63 +378,89 @@ public class TaskPersister {
    }

    private class LazyTaskWriterThread extends Thread {
        boolean mSlow = true;
        LinkedList<BitmapQueueEntry> mSaveImagesQueue = new LinkedList<BitmapQueueEntry>();

        LazyTaskWriterThread(String name) {
            super(name);
        }

        class BitmapQueueEntry {
            final Bitmap mImage;
            final String mFilename;
            BitmapQueueEntry(Bitmap image, String filename) {
                mImage = image;
                mFilename = filename;
        @Override
        public void run() {
            ArraySet<Integer> persistentTaskIds = new ArraySet<Integer>();
            while (true) {
                // We can't lock mService while holding TaskPersister.this, but we don't want to
                // call removeObsoleteFiles every time through the loop, only the last time before
                // going to sleep. The risk is that we call removeObsoleteFiles() successively.
                final boolean probablyDone;
                synchronized (TaskPersister.this) {
                    probablyDone = mWriteQueue.isEmpty();
                }
                if (probablyDone) {
                    if (DEBUG) Slog.d(TAG, "Looking for obsolete files.");
                    persistentTaskIds.clear();
                    synchronized (mService) {
                        final ArrayList<TaskRecord> tasks = mService.mRecentTasks;
                        if (DEBUG) Slog.d(TAG, "mRecents=" + tasks);
                        for (int taskNdx = tasks.size() - 1; taskNdx >= 0; --taskNdx) {
                            final TaskRecord task = tasks.get(taskNdx);
                            if (DEBUG) Slog.d(TAG, "LazyTaskWriter: task=" + task + " persistable=" +
                                    task.isPersistable);
                            if (task.isPersistable && !task.stack.isHomeStack()) {
                                if (DEBUG) Slog.d(TAG, "adding to persistentTaskIds task=" + task);
                                persistentTaskIds.add(task.taskId);
                            } else {
                                if (DEBUG) Slog.d(TAG, "omitting from persistentTaskIds task=" + task);
                            }
                        }

        void saveImage(Bitmap image, String filename) {
            synchronized (mSaveImagesQueue) {
                mSaveImagesQueue.add(new BitmapQueueEntry(image, filename));
                    }
            TaskPersister.this.notify(null, false);
                    removeObsoleteFiles(persistentTaskIds);
                }

        @Override
        public void run() {
            ArraySet<Integer> persistentTaskIds = new ArraySet<Integer>();
            while (true) {
                // If mSlow, then delay between each call to saveToXml().
                // If mNextWriteTime, then don't delay between each call to saveToXml().
                final WriteQueueItem item;
                synchronized (TaskPersister.this) {
                    if (mNextWriteTime >= 0) {
                        // The next write we don't have to wait so long.
                        mNextWriteTime = SystemClock.uptimeMillis() + INTER_WRITE_DELAY_MS;
                        if (DEBUG) Slog.d(TAG, "Next write time may be in " +
                                INTER_WRITE_DELAY_MS + " msec. (" + mNextWriteTime + ")");
                    }

                    while (mWriteQueue.isEmpty()) {
                        mNextWriteTime = 0; // idle.
                        try {
                            if (DEBUG) Slog.d(TAG, "LazyTaskWriter: waiting indefinitely.");
                            TaskPersister.this.wait();
                        } catch (InterruptedException e) {
                        }
                        // Invariant: mNextWriteTime is either -1 or PRE_WRITE_DELAY_MS from now.
                    }
                    item = mWriteQueue.remove(0);

                    long now = SystemClock.uptimeMillis();
                    final long releaseTime =
                            now + (DEBUG ? DEBUG_INTER_TASK_DELAY_MS: INTER_TASK_DELAY_MS);
                    while (mSlow && now < releaseTime) {
                    if (DEBUG) Slog.d(TAG, "LazyTaskWriter: now=" + now + " mNextWriteTime=" +
                            mNextWriteTime);
                    while (now < mNextWriteTime) {
                        try {
                            if (DEBUG) Slog.d(TAG, "LazyTaskWriter: waiting " +
                                    (releaseTime - now));
                            TaskPersister.this.wait(releaseTime - now);
                                    (mNextWriteTime - now));
                            TaskPersister.this.wait(mNextWriteTime - now);
                        } catch (InterruptedException e) {
                        }
                        now = SystemClock.uptimeMillis();
                    }
                }

                // Write out one bitmap that needs saving each time through.
                BitmapQueueEntry entry;
                synchronized (mSaveImagesQueue) {
                    entry = mSaveImagesQueue.poll();
                    // Are there any more after this one?
                    mRecentsChanged |= !mSaveImagesQueue.isEmpty();
                    // Got something to do.
                }
                if (entry != null) {
                    final String filename = entry.mFilename;
                    if (DEBUG) Slog.d(TAG, "saveImage: filename=" + filename);

                if (item instanceof ImageWriteQueueItem) {
                    ImageWriteQueueItem imageWriteQueueItem = (ImageWriteQueueItem) item;
                    final String filename = imageWriteQueueItem.mFilename;
                    final Bitmap bitmap = imageWriteQueueItem.mImage;
                    if (DEBUG) Slog.d(TAG, "writing bitmap: filename=" + filename);
                    FileOutputStream imageFile = null;
                    try {
                        imageFile = new FileOutputStream(new File(sImagesDir, filename));
                        entry.mImage.compress(Bitmap.CompressFormat.PNG, 100, imageFile);
                        bitmap.compress(Bitmap.CompressFormat.PNG, 100, imageFile);
                    } catch (Exception e) {
                        Slog.e(TAG, "saveImage: unable to save " + filename, e);
                    } finally {
@@ -362,46 +471,28 @@ public class TaskPersister {
                            }
                        }
                    }
                }

                } else if (item instanceof TaskWriteQueueItem) {
                    // Write out one task.
                    StringWriter stringWriter = null;
                TaskRecord task = null;
                    TaskRecord task = ((TaskWriteQueueItem) item).mTask;
                    if (DEBUG) Slog.d(TAG, "Writing task=" + task);
                    synchronized (mService) {
                    final ArrayList<TaskRecord> tasks = mService.mRecentTasks;
                    persistentTaskIds.clear();
                    if (DEBUG) Slog.d(TAG, "mRecents=" + tasks);
                    for (int taskNdx = tasks.size() - 1; taskNdx >= 0; --taskNdx) {
                        task = tasks.get(taskNdx);
                        if (DEBUG) Slog.d(TAG, "LazyTaskWriter: task=" + task + " persistable=" +
                                task.isPersistable + " needsPersisting=" + task.needsPersisting);
                        if (task.isPersistable && !task.stack.isHomeStack()) {
                            if (DEBUG) Slog.d(TAG, "adding to persistentTaskIds task=" + task);
                            persistentTaskIds.add(task.taskId);

                            if (task.needsPersisting) {
                        if (mService.mRecentTasks.contains(task)) {
                            // Still there.
                            try {
                                if (DEBUG) Slog.d(TAG, "Saving task=" + task);
                                stringWriter = saveToXml(task);
                                    break;
                            } catch (IOException e) {
                            } catch (XmlPullParserException e) {
                                } finally {
                                    task.needsPersisting = false;
                                }
                            }
                        } else {
                            if (DEBUG) Slog.d(TAG, "omitting from persistentTaskIds task=" + task);
                        }
                            }
                        }

                        if (stringWriter != null) {
                            // Write out xml file while not holding mService lock.
                            FileOutputStream file = null;
                            AtomicFile atomicFile = null;
                            try {
                        atomicFile = new AtomicFile(new File(sTasksDir,
                                String.valueOf(task.taskId) + RECENTS_FILENAME + TASK_EXTENSION));
                                atomicFile = new AtomicFile(new File(sTasksDir, String.valueOf(
                                        task.taskId) + RECENTS_FILENAME + TASK_EXTENSION));
                                file = atomicFile.startWrite();
                                file.write(stringWriter.toString().getBytes());
                                file.write('\n');
@@ -410,31 +501,12 @@ public class TaskPersister {
                                if (file != null) {
                                    atomicFile.failWrite(file);
                                }
                        Slog.e(TAG, "Unable to open " + atomicFile + " for persisting. " + e);
                    }
                } else {
                    // Made it through the entire list and didn't find anything new that needed
                    // persisting.
                    if (!DEBUG) {
                        if (DEBUG) Slog.d(TAG, "Calling removeObsoleteFiles persistentTaskIds=" +
                                persistentTaskIds);
                        removeObsoleteFiles(persistentTaskIds);
                    }

                    // Wait here for someone to call setRecentsChanged().
                    synchronized (TaskPersister.this) {
                        while (!mRecentsChanged) {
                            if (DEBUG) Slog.d(TAG, "LazyTaskWriter: Waiting.");
                            try {
                                TaskPersister.this.wait();
                            } catch (InterruptedException e) {
                                Slog.e(TAG, "Unable to open " + atomicFile + " for persisting. " +
                                        e);
                            }
                        }
                        mRecentsChanged = false;
                        if (DEBUG) Slog.d(TAG, "LazyTaskWriter: Awake");
                    }
                }
                // Some recents file needs to be written.
            }
        }
    }
+3 −3
Original line number Diff line number Diff line
@@ -122,9 +122,6 @@ final class TaskRecord {
     * (positive) or back (negative). Absolute value indicates time. */
    long mLastTimeMoved = System.currentTimeMillis();

    /** True if persistable, has changed, and has not yet been persisted */
    boolean needsPersisting = false;

    /** Indication of what to run next when task exits. Use ActivityRecord types.
     * ActivityRecord.APPLICATION_ACTIVITY_TYPE indicates to resume the task below this one in the
     * task stack. */
@@ -362,6 +359,9 @@ final class TaskRecord {
    void getLastThumbnail(TaskThumbnail thumbs) {
        thumbs.mainThumbnail = mLastThumbnail;
        thumbs.thumbnailFileDescriptor = null;
        if (mLastThumbnail == null) {
            thumbs.mainThumbnail = mService.mTaskPersister.getThumbnail(mFilename);
        }
        if (mLastThumbnailFile.exists()) {
            try {
                thumbs.thumbnailFileDescriptor = ParcelFileDescriptor.open(mLastThumbnailFile,