commit cf76a16e660a361670745b043198b797ccd86747
author Daniel Nishi <dhnishi@google.com> Thu Dec 22 11:06:51 2016 -0800
committer Daniel Nishi <dhnishi@google.com> Thu Dec 22 11:32:33 2016 -0800
tree 48871860c02a0cb43faf49fa59dc5b47b0e5f6aa
parent 683dbf1fc6d28ba5008779627bf4faf8cd14106c

Fix a bug where the package stats query timeout may crash.

This adds a null-check to verify that we got real data before
trying to use it and increases the timeout time to reduce the
likelihood of timing out.

Test: FrameworkServicesTests
Bug: 33836034
Change-Id: Ia1ad4aba05c5c4fb8688fc9fc94df344f736e396

services/core/java/com/android/server/storage/DiskStatsLoggingService.java

diff --git a/services/core/java/com/android/server/storage/DiskStatsLoggingService.java b/services/core/java/com/android/server/storage/DiskStatsLoggingService.java
index 4a86175..0a3abf3 100644
--- a/services/core/java/com/android/server/storage/DiskStatsLoggingService.java
+++ b/services/core/java/com/android/server/storage/DiskStatsLoggingService.java
@@ -106,7 +106,7 @@
 
     @VisibleForTesting
     static class LogRunnable implements Runnable {
-        private static final long TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(5);
+        private static final long TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(10);
 
         private JobService mJobService;
         private JobParameters mParams;
@@ -147,11 +147,17 @@
             FileCollector.MeasurementResult downloads =
                     FileCollector.getMeasurementResult(mDownloadsDirectory);
 
-            logToFile(mainCategories, downloads, mCollector.getPackageStats(TIMEOUT_MILLIS),
-                    mSystemSize);
+            boolean needsReschedule = true;
+            List<PackageStats> stats = mCollector.getPackageStats(TIMEOUT_MILLIS);
+            if (stats != null) {
+                needsReschedule = false;
+                logToFile(mainCategories, downloads, stats, mSystemSize);
+            } else {
+                Log.w("TAG", "Timed out while fetching package stats.");
+            }
 
             if (mJobService != null) {
-                mJobService.jobFinished(mParams, false);
+                mJobService.jobFinished(mParams, needsReschedule);
             }
         }
 

services/tests/servicestests/src/com/android/server/storage/DiskStatsLoggingServiceTest.java

diff --git a/services/tests/servicestests/src/com/android/server/storage/DiskStatsLoggingServiceTest.java b/services/tests/servicestests/src/com/android/server/storage/DiskStatsLoggingServiceTest.java
index 357ce74..3789086 100644
--- a/services/tests/servicestests/src/com/android/server/storage/DiskStatsLoggingServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/storage/DiskStatsLoggingServiceTest.java
@@ -18,9 +18,15 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import android.app.job.JobService;
+import android.app.job.JobParameters;
 import android.content.pm.PackageStats;
 import android.test.AndroidTestCase;
 
@@ -130,6 +136,21 @@
         assertThat(json.getJSONArray(DiskStatsFileLogger.APP_CACHES_KEY).length()).isEqualTo(1L);
     }
 
+    @Test
+    public void testDontCrashOnPackageStatsTimeout() throws Exception {
+        when(mCollector.getPackageStats(anyInt())).thenReturn(null);
+
+        LogRunnable task = new LogRunnable();
+        task.setAppCollector(mCollector);
+        task.setDownloadsDirectory(mDownloads.getRoot());
+        task.setRootDirectory(mRootDirectory.getRoot());
+        task.setLogOutputFile(mInputFile);
+        task.setSystemSize(10L);
+        task.run();
+
+        // No exception should be thrown.
+    }
+
     private void writeDataToFile(File f, String data) throws Exception{
         PrintStream out = new PrintStream(f);
         out.print(data);