Purge StoreWriteQueue items to avoid system health issues

If queue gets too deep we may run out of memory or cause other
system health issues.

Test: TaskSnapshotPersisterLoaderTest
Bug: 38416992
Bug: 37631016
Change-Id: I725c9a458f78af2e625f2451bb0030176035f596

diff --git a/services/core/java/com/android/server/wm/TaskSnapshotPersister.java b/services/core/java/com/android/server/wm/TaskSnapshotPersister.java
index 866bfc0..297e288 100644 (file)
--- a/services/core/java/com/android/server/wm/TaskSnapshotPersister.java
+++ b/services/core/java/com/android/server/wm/TaskSnapshotPersister.java
@@ -39,6 +39,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 
 /**
  * Persists {@link TaskSnapshot}s to disk.
@@ -55,10 +56,13 @@ class TaskSnapshotPersister {
     private static final int QUALITY = 95;
     private static final String PROTO_EXTENSION = ".proto";
     private static final String BITMAP_EXTENSION = ".jpg";
+    private static final int MAX_STORE_QUEUE_DEPTH = 2;
 
     @GuardedBy("mLock")
     private final ArrayDeque<WriteQueueItem> mWriteQueue = new ArrayDeque<>();
     @GuardedBy("mLock")
+    private final ArrayDeque<StoreWriteQueueItem> mStoreQueueItems = new ArrayDeque<>();
+    @GuardedBy("mLock")
     private boolean mQueueIdling;
     @GuardedBy("mLock")
     private boolean mPaused;
@@ -153,11 +157,22 @@ class TaskSnapshotPersister {
     @GuardedBy("mLock")
     private void sendToQueueLocked(WriteQueueItem item) {
         mWriteQueue.offer(item);
+        item.onQueuedLocked();
+        ensureStoreQueueDepthLocked();
         if (!mPaused) {
             mLock.notifyAll();
         }
     }
 
+    @GuardedBy("mLock")
+    private void ensureStoreQueueDepthLocked() {
+        while (mStoreQueueItems.size() > MAX_STORE_QUEUE_DEPTH) {
+            final StoreWriteQueueItem item = mStoreQueueItems.poll();
+            mWriteQueue.remove(item);
+            Slog.i(TAG, "Queue is too deep! Purged item with taskid=" + item.mTaskId);
+        }
+    }
+
     private File getDirectory(int userId) {
         return new File(mDirectoryResolver.getSystemDirectoryForUser(userId), SNAPSHOTS_DIRNAME);
     }
@@ -202,6 +217,9 @@ class TaskSnapshotPersister {
                         next = null;
                     } else {
                         next = mWriteQueue.poll();
+                        if (next != null) {
+                            next.onDequeuedLocked();
+                        }
                     }
                 }
                 if (next != null) {
@@ -226,6 +244,18 @@ class TaskSnapshotPersister {
 
     private abstract class WriteQueueItem {
         abstract void write();
+
+        /**
+         * Called when this queue item has been put into the queue.
+         */
+        void onQueuedLocked() {
+        }
+
+        /**
+         * Called when this queue item has been taken out of the queue.
+         */
+        void onDequeuedLocked() {
+        }
     }
 
     private class StoreWriteQueueItem extends WriteQueueItem {
@@ -240,6 +270,16 @@ class TaskSnapshotPersister {
         }
 
         @Override
+        void onQueuedLocked() {
+            mStoreQueueItems.offer(this);
+        }
+
+        @Override
+        void onDequeuedLocked() {
+            mStoreQueueItems.remove(this);
+        }
+
+        @Override
         void write() {
             if (!createDirectory(mUserId)) {
                 Slog.e(TAG, "Unable to create snapshot directory for user dir="

diff --git a/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotPersisterLoaderTest.java b/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotPersisterLoaderTest.java
index 8108909..39c0de8 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotPersisterLoaderTest.java
+++ b/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotPersisterLoaderTest.java
@@ -90,12 +90,40 @@ public class TaskSnapshotPersisterLoaderTest extends TaskSnapshotPersisterTestBa
         long ms = SystemClock.elapsedRealtime();
         mPersister.persistSnapshot(1, mTestUserId, createSnapshot());
         mPersister.persistSnapshot(2, mTestUserId, createSnapshot());
+        mPersister.removeObsoleteFiles(new ArraySet<>(), new int[] { mTestUserId });
+        mPersister.removeObsoleteFiles(new ArraySet<>(), new int[] { mTestUserId });
+        mPersister.removeObsoleteFiles(new ArraySet<>(), new int[] { mTestUserId });
+        mPersister.removeObsoleteFiles(new ArraySet<>(), new int[] { mTestUserId });
+        mPersister.waitForQueueEmpty();
+        assertTrue(SystemClock.elapsedRealtime() - ms > 500);
+    }
+
+    /**
+     * Tests that too many store write queue items are being purged.
+     */
+    @Test
+    public void testPurging() {
+        mPersister.persistSnapshot(100, mTestUserId, createSnapshot());
+        mPersister.waitForQueueEmpty();
+        mPersister.setPaused(true);
+        mPersister.persistSnapshot(1, mTestUserId, createSnapshot());
+        mPersister.removeObsoleteFiles(new ArraySet<>(), new int[] { mTestUserId });
+        mPersister.persistSnapshot(2, mTestUserId, createSnapshot());
         mPersister.persistSnapshot(3, mTestUserId, createSnapshot());
         mPersister.persistSnapshot(4, mTestUserId, createSnapshot());
-        mPersister.persistSnapshot(5, mTestUserId, createSnapshot());
-        mPersister.persistSnapshot(6, mTestUserId, createSnapshot());
+        mPersister.setPaused(false);
         mPersister.waitForQueueEmpty();
-        assertTrue(SystemClock.elapsedRealtime() - ms > 500);
+
+        // Make sure 1,2 were purged but removeObsoleteFiles wasn't.
+        final File[] existsFiles = new File[] {
+                new File(sFilesDir.getPath() + "/snapshots/3.proto"),
+                new File(sFilesDir.getPath() + "/snapshots/4.proto")};
+        final File[] nonExistsFiles = new File[] {
+                new File(sFilesDir.getPath() + "/snapshots/100.proto"),
+                new File(sFilesDir.getPath() + "/snapshots/1.proto"),
+                new File(sFilesDir.getPath() + "/snapshots/1.proto")};
+        assertTrueForFiles(existsFiles, File::exists, " must exist");
+        assertTrueForFiles(nonExistsFiles, file -> !file.exists(), " must not exist");
     }
 
     @Test