Merge "Remove corrupt apexes only if not active" into rvc-dev

2 files changed, 65 insertions, 61 deletions

diff --git a/apexd/apexd.cpp b/apexd/apexd.cpp
index 5c0a4711..a2f56cf6 100644
--- a/apexd/apexd.cpp
+++ b/apexd/apexd.cpp
@@ -908,14 +908,15 @@ Result<void> Unmount(const MountedApexData& data) {
   return android::apex::Unmount(data);
 }
 
-bool IsMounted(const std::string& name, const std::string& full_path) {
+bool IsMounted(const std::string& full_path) {
   bool found_mounted = false;
-  gMountedApexes.ForallMountedApexes(
-      name, [&](const MountedApexData& data, bool latest ATTRIBUTE_UNUSED) {
-        if (full_path == data.full_path) {
-          found_mounted = true;
-        }
-      });
+  gMountedApexes.ForallMountedApexes([&](const std::string&,
+                                         const MountedApexData& data,
+                                         [[maybe_unused]] bool latest) {
+    if (full_path == data.full_path) {
+      found_mounted = true;
+    }
+  });
   return found_mounted;
 }
 
@@ -2116,9 +2117,14 @@ void RemoveOrphanedApexes() {
   for (const auto& path : *data_apexes) {
     auto apex = ApexFile::Open(path);
     if (!apex.ok()) {
-      LOG(DEBUG) << "Removing corrupt APEX " << path << " : " << apex.error();
-      if (unlink(path.c_str()) != 0) {
-        PLOG(ERROR) << "Failed to unlink " << path;
+      LOG(DEBUG) << "Failed to open APEX " << path << " : " << apex.error();
+      // before removing, double-check if the path is active or not
+      // just in case ApexFile::Open() fails with valid APEX
+      if (!apexd_private::IsMounted(path)) {
+        LOG(DEBUG) << "Removing corrupt APEX " << path;
+        if (unlink(path.c_str()) != 0) {
+          PLOG(ERROR) << "Failed to unlink " << path;
+        }
       }
       continue;
     }
diff --git a/tests/src/com/android/tests/apex/ApexdHostTest.java b/tests/src/com/android/tests/apex/ApexdHostTest.java
index 2998ac81..fc1fa4be 100644
--- a/tests/src/com/android/tests/apex/ApexdHostTest.java
+++ b/tests/src/com/android/tests/apex/ApexdHostTest.java
@@ -26,7 +26,6 @@ import com.android.tradefed.device.ITestDevice;
 import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
 import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
 
-import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -122,20 +121,39 @@ public class ApexdHostTest extends BaseHostJUnit4Test  {
         assumeTrue("Device does not support updating APEX", mTestUtils.isApexUpdateSupported());
         assumeTrue("Device requires root", getDevice().isAdbRoot());
 
-        withTestFiles(new String[]{
-                "apex.apexd_test.apex", "/product/apex",
-                "apex.apexd_test_v2_no_pb.apex", "/data/apex/active"
-        }, () -> {
-                final Set<ITestDevice.ApexInfo> activeApexes = getDevice().getActiveApexes();
-                assertThat(activeApexes).contains(new ITestDevice.ApexInfo(
-                        "com.android.apex.test_package", 1L));
-                assertThat(activeApexes).doesNotContain(new ITestDevice.ApexInfo(
-                        "com.android.apex.test_package", 2L));
-
-                // v2_no_pb should be deleted
-                mTestUtils.waitForFileDeleted("/data/apex/active/apex.apexd_test_v2_no_pb.apex",
-                        Duration.ofMinutes(3));
-            });
+        try {
+            getDevice().remountSystemWritable();
+            // In case remount requires a reboot, wait for boot to complete.
+            assertWithMessage("Timed out waiting for device to boot").that(
+                    getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
+
+            final File v1 = mTestUtils.getTestFile("apex.apexd_test.apex");
+            getDevice().pushFile(v1, "/product/apex/apex.apexd_test.apex");
+
+            final File v2_no_pb = mTestUtils.getTestFile("apex.apexd_test_v2_no_pb.apex");
+            getDevice().pushFile(v2_no_pb, "/data/apex/active/apex.apexd_test_v2_no_pb.apex");
+
+            getDevice().reboot();
+            assertWithMessage("Timed out waiting for device to boot").that(
+                    getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
+
+            final Set<ITestDevice.ApexInfo> activeApexes = getDevice().getActiveApexes();
+            assertThat(activeApexes).contains(new ITestDevice.ApexInfo(
+                    "com.android.apex.test_package", 1L));
+            assertThat(activeApexes).doesNotContain(new ITestDevice.ApexInfo(
+                    "com.android.apex.test_package", 2L));
+
+            // v2_no_pb should be deleted
+            mTestUtils.waitForFileDeleted("/data/apex/active/apex.apexd_test_v2_no_pb.apex",
+                    Duration.ofMinutes(3));
+        } finally {
+            getDevice().remountSystemWritable();
+            assertWithMessage("Timed out waiting for device to boot").that(
+                    getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
+
+            getDevice().executeShellV2Command("rm /product/apex/apex.apexd_test.apex");
+            getDevice().executeShellV2Command("rm /data/apex/active/apex.apexd_test_v2_no_pb.apex");
+        }
     }
 
     @Test
@@ -144,58 +162,38 @@ public class ApexdHostTest extends BaseHostJUnit4Test  {
         assumeTrue("Device does not support updating APEX", mTestUtils.isApexUpdateSupported());
         assumeTrue("Device requires root", getDevice().isAdbRoot());
 
-        withTestFiles(new String[]{
-                "apex.apexd_test_v3.apex", "/product/apex",
-                "apex.apexd_test_v2_no_pb.apex", "/data/apex/active"
-        }, () -> {
-                final Set<ITestDevice.ApexInfo> activeApexes = getDevice().getActiveApexes();
-                assertThat(activeApexes).contains(new ITestDevice.ApexInfo(
-                        "com.android.apex.test_package", 3L));
-                assertThat(activeApexes).doesNotContain(new ITestDevice.ApexInfo(
-                        "com.android.apex.test_package", 2L));
-
-                // v2_no_pb should be deleted
-                mTestUtils.waitForFileDeleted("/data/apex/active/apex.apexd_test_v2_no_pb.apex",
-                        Duration.ofMinutes(3));
-            });
-    }
-
-    private interface ThrowingRunnable {
-        void run() throws Exception;
-    }
-
-    private void withTestFiles(String[] files, ThrowingRunnable body) throws Exception {
-        Assert.assertTrue("files should have even elements", files.length % 2 == 0);
         try {
             getDevice().remountSystemWritable();
             // In case remount requires a reboot, wait for boot to complete.
             assertWithMessage("Timed out waiting for device to boot").that(
                     getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
 
-            // copy test files
-            for (int i = 0; i < files.length; i += 2) {
-                final String filename = files[i];
-                final String path = files[i + 1];
-                final File file = mTestUtils.getTestFile(filename);
-                getDevice().pushFile(file,  path + "/" + filename);
-            }
+            final File v3 = mTestUtils.getTestFile("apex.apexd_test_v3.apex");
+            getDevice().pushFile(v3, "/product/apex/apex.apexd_test_v3.apex");
+
+            final File v2_no_pb = mTestUtils.getTestFile("apex.apexd_test_v2_no_pb.apex");
+            getDevice().pushFile(v2_no_pb, "/data/apex/active/apex.apexd_test_v2_no_pb.apex");
 
             getDevice().reboot();
             assertWithMessage("Timed out waiting for device to boot").that(
                     getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
 
-            body.run();
+            final Set<ITestDevice.ApexInfo> activeApexes = getDevice().getActiveApexes();
+            assertThat(activeApexes).contains(new ITestDevice.ApexInfo(
+                    "com.android.apex.test_package", 3L));
+            assertThat(activeApexes).doesNotContain(new ITestDevice.ApexInfo(
+                    "com.android.apex.test_package", 2L));
+
+            // v2_no_pb should be deleted
+            mTestUtils.waitForFileDeleted("/data/apex/active/apex.apexd_test_v2_no_pb.apex",
+                    Duration.ofMinutes(3));
         } finally {
             getDevice().remountSystemWritable();
             assertWithMessage("Timed out waiting for device to boot").that(
                     getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
 
-            // remove test files
-            for (int i = 0; i < files.length; i += 2) {
-                final String filename = files[i];
-                final String path = files[i + 1];
-                getDevice().executeShellV2Command("rm " + path + "/" + filename);
-            }
+            getDevice().executeShellV2Command("rm /product/apex/apex.apexd_test_v3.apex");
+            getDevice().executeShellV2Command("rm /data/apex/active/apex.apexd_test_v2_no_pb.apex");
         }
     }
 }