From 1893959cf1d0b56de0f6ba98fd308e38c7ff1eb6 Mon Sep 17 00:00:00 2001
From: Rhed Jao <rhedjao@google.com>
Date: Mon, 7 Dec 2020 19:44:13 +0800
Subject: [PATCH] Returns immediately if the bugreport file already exists

There's a case that BugreportProgressService is invoked twice quickly,
and both services create the same bugreport file name. The later one
may delete current running bugreport file in its clean function,
when it detects another bugreport is running.

Bug: 174314124
Test: atest BugreportReceiverTest
Change-Id: I5e1802c5912f4414f1ad3b8bdaf7c7420332b9d6
---
 .../android/shell/BugreportProgressService.java   | 15 +++++++++++----
 .../com/android/shell/BugreportReceiverTest.java  | 13 +++++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java
index d1a0b49014ce0..a2608a0a30570 100644
--- a/packages/Shell/src/com/android/shell/BugreportProgressService.java
+++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java
@@ -618,12 +618,21 @@ public class BugreportProgressService extends Service {
 
         BugreportInfo info = new BugreportInfo(mContext, baseName, name,
                 shareTitle, shareDescription, bugreportType, mBugreportsDir);
+        synchronized (mLock) {
+            if (info.bugreportFile.exists()) {
+                Log.e(TAG, "Failed to start bugreport generation, the requested bugreport file "
+                        + info.bugreportFile + " already exists");
+                return;
+            }
+            info.createBugreportFile();
+        }
         ParcelFileDescriptor bugreportFd = info.getBugreportFd();
         if (bugreportFd == null) {
             Log.e(TAG, "Failed to start bugreport generation as "
                     + " bugreport parcel file descriptor is null.");
             return;
         }
+        info.createScreenshotFile(mBugreportsDir);
         ParcelFileDescriptor screenshotFd = null;
         if (isDefaultScreenshotRequired(bugreportType, /* hasScreenshotButton= */ !mIsTv)) {
             screenshotFd = info.getDefaultScreenshotFd();
@@ -1920,12 +1929,10 @@ public class BugreportProgressService extends Service {
             this.shareDescription = shareDescription == null ? "" : shareDescription;
             this.type = type;
             this.baseName = baseName;
-            createBugreportFile(bugreportsDir);
-            createScreenshotFile(bugreportsDir);
+            this.bugreportFile = new File(bugreportsDir, getFileName(this, ".zip"));
         }
 
-        void createBugreportFile(File bugreportsDir) {
-            bugreportFile = new File(bugreportsDir, getFileName(this, ".zip"));
+        void createBugreportFile() {
             createReadWriteFile(bugreportFile);
         }
 
diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
index 89cdeaea199c7..9476912067417 100644
--- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
+++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java
@@ -40,6 +40,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
 import android.app.ActivityManager;
@@ -534,6 +535,18 @@ public class BugreportReceiverTest {
         assertActionSendMultiple(extras);
     }
 
+    @Test
+    public void testBugreportRequestTwice_oneStartBugreportInvoked() throws Exception {
+        sendBugreportStarted();
+        new BugreportRequestedReceiver().onReceive(mContext,
+                new Intent(INTENT_BUGREPORT_REQUESTED));
+        getInstrumentation().waitForIdleSync();
+
+        verify(mMockIDumpstate, times(1)).startBugreport(anyInt(), any(), any(), any(),
+                anyInt(), any(), anyBoolean());
+        sendBugreportFinished();
+    }
+
     private void cancelExistingNotifications() {
         // Must kill service first, because notifications from a foreground service cannot be
         // canceled.