From ef3651cff41b929b2fc0c3bd6effe93257f4d831 Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Thu, 18 May 2017 23:58:09 +0200 Subject: [PATCH] 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 --- .../server/wm/TaskSnapshotPersister.java | 40 +++++++++++++++++++ .../wm/TaskSnapshotPersisterLoaderTest.java | 36 +++++++++++++++-- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/wm/TaskSnapshotPersister.java b/services/core/java/com/android/server/wm/TaskSnapshotPersister.java index 866bfc015a5f4..297e2880a4554 100644 --- 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 mWriteQueue = new ArrayDeque<>(); @GuardedBy("mLock") + private final ArrayDeque 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 { @@ -239,6 +269,16 @@ class TaskSnapshotPersister { mSnapshot = snapshot; } + @Override + void onQueuedLocked() { + mStoreQueueItems.offer(this); + } + + @Override + void onDequeuedLocked() { + mStoreQueueItems.remove(this); + } + @Override void write() { if (!createDirectory(mUserId)) { 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 8108909152bbd..39c0de82e821b 100644 --- a/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotPersisterLoaderTest.java +++ b/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotPersisterLoaderTest.java @@ -90,14 +90,42 @@ public class TaskSnapshotPersisterLoaderTest extends TaskSnapshotPersisterTestBa long ms = SystemClock.elapsedRealtime(); mPersister.persistSnapshot(1, mTestUserId, createSnapshot()); 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.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.setPaused(false); + mPersister.waitForQueueEmpty(); + + // 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 public void testGetTaskId() { RemoveObsoleteFilesQueueItem removeObsoleteFilesQueueItem =