Merge "Revert "Add crash detection and recovery."" into sc-dev

6 files changed, 6 insertions, 226 deletions

diff --git a/services/core/java/com/android/server/graphics/fonts/FontCrashDetector.java b/services/core/java/com/android/server/graphics/fonts/FontCrashDetector.java
deleted file mode 100644
index b082b25aea02..000000000000
--- a/services/core/java/com/android/server/graphics/fonts/FontCrashDetector.java
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.server.graphics.fonts;
-
-import android.annotation.NonNull;
-import android.util.Slog;
-
-import java.io.File;
-import java.io.IOException;
-
-/**
- * A class to detect font-related native crash.
- *
- * <p>If a fs-verity protected file is accessed through mmap and corrupted file block is detected,
- * SIGBUG signal is generated and the process will crash. To find corrupted files and remove them,
- * we use a marker file to detect crash.
- * <ol>
- *     <li>Create a marker file before reading fs-verity protected font files.
- *     <li>Delete the marker file after reading font files successfully.
- *     <li>If the marker file is found in the next process startup, it means that the process
- *         crashed before. We will delete font files to prevent crash loop.
- * </ol>
- *
- * <p>Example usage:
- * <pre>
- *     FontCrashDetector detector = new FontCrashDetector(new File("/path/to/marker_file"));
- *     if (detector.hasCrashed()) {
- *         // Do cleanup
- *     }
- *     try (FontCrashDetector.MonitoredBlock b = detector.start()) {
- *         // Read files
- *     }
- * </pre>
- *
- * <p>This class DOES NOT detect Java exceptions. If a Java exception is thrown while monitoring
- * crash, the marker file will be deleted. Creating and deleting marker files are not lightweight.
- * Please use this class sparingly with caution.
- */
-/* package */ final class FontCrashDetector {
-
-    private static final String TAG = "FontCrashDetector";
-
-    @NonNull
-    private final File mMarkerFile;
-
-    /* package */ FontCrashDetector(@NonNull File markerFile) {
-        mMarkerFile = markerFile;
-    }
-
-    /* package */ boolean hasCrashed() {
-        return mMarkerFile.exists();
-    }
-
-    /* package */ void clear() {
-        if (!mMarkerFile.delete()) {
-            Slog.e(TAG, "Could not delete marker file: " + mMarkerFile);
-        }
-    }
-
-    /** Starts crash monitoring. */
-    /* package */ MonitoredBlock start() {
-        try {
-            mMarkerFile.createNewFile();
-        } catch (IOException e) {
-            Slog.e(TAG, "Could not create marker file: " + mMarkerFile, e);
-        }
-        return new MonitoredBlock();
-    }
-
-    /** A helper class to monitor crash with try-with-resources syntax. */
-    /* package */ class MonitoredBlock implements AutoCloseable {
-        /** Ends crash monitoring. */
-        @Override
-        public void close() {
-            clear();
-        }
-    }
-}
diff --git a/services/core/java/com/android/server/graphics/fonts/FontManagerService.java b/services/core/java/com/android/server/graphics/fonts/FontManagerService.java
index 01e839dae07a..900ec905609f 100644
--- a/services/core/java/com/android/server/graphics/fonts/FontManagerService.java
+++ b/services/core/java/com/android/server/graphics/fonts/FontManagerService.java
@@ -63,7 +63,6 @@ public final class FontManagerService extends IFontManager.Stub {
     private static final String TAG = "FontManagerService";
 
     private static final String FONT_FILES_DIR = "/data/fonts/files";
-    private static final String CRASH_MARKER_FILE = "/data/fonts/config/crash.txt";
 
     @Override
     public FontConfig getFontConfig() {
@@ -200,10 +199,6 @@ public final class FontManagerService extends IFontManager.Stub {
     private final Object mUpdatableFontDirLock = new Object();
 
     @GuardedBy("mUpdatableFontDirLock")
-    @NonNull
-    private final FontCrashDetector mFontCrashDetector;
-
-    @GuardedBy("mUpdatableFontDirLock")
     @Nullable
     private final UpdatableFontDir mUpdatableFontDir;
 
@@ -217,7 +212,6 @@ public final class FontManagerService extends IFontManager.Stub {
 
     private FontManagerService(Context context) {
         mContext = context;
-        mFontCrashDetector = new FontCrashDetector(new File(CRASH_MARKER_FILE));
         mUpdatableFontDir = createUpdatableFontDir();
         initialize();
     }
@@ -244,19 +238,8 @@ public final class FontManagerService extends IFontManager.Stub {
                 }
                 return;
             }
-            if (mFontCrashDetector.hasCrashed()) {
-                Slog.i(TAG, "Crash detected. Clearing font updates.");
-                try {
-                    mUpdatableFontDir.clearUpdates();
-                } catch (SystemFontException e) {
-                    Slog.e(TAG, "Failed to clear updates.", e);
-                }
-                mFontCrashDetector.clear();
-            }
-            try (FontCrashDetector.MonitoredBlock ignored = mFontCrashDetector.start()) {
-                mUpdatableFontDir.loadFontFileMap();
-                updateSerializedFontMap();
-            }
+            mUpdatableFontDir.loadFontFileMap();
+            updateSerializedFontMap();
         }
     }
 
@@ -286,10 +269,8 @@ public final class FontManagerService extends IFontManager.Stub {
                         FontManager.RESULT_ERROR_VERSION_MISMATCH,
                         "The base config version is older than current.");
             }
-            try (FontCrashDetector.MonitoredBlock ignored = mFontCrashDetector.start()) {
-                mUpdatableFontDir.update(requests);
-                updateSerializedFontMap();
-            }
+            mUpdatableFontDir.update(requests);
+            updateSerializedFontMap();
         }
     }
 
@@ -300,10 +281,8 @@ public final class FontManagerService extends IFontManager.Stub {
                     "The font updater is disabled.");
         }
         synchronized (mUpdatableFontDirLock) {
-            try (FontCrashDetector.MonitoredBlock ignored = mFontCrashDetector.start()) {
-                mUpdatableFontDir.clearUpdates();
-                updateSerializedFontMap();
-            }
+            mUpdatableFontDir.clearUpdates();
+            updateSerializedFontMap();
         }
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/graphics/fonts/FontCrashDetectorTest.java b/services/tests/servicestests/src/com/android/server/graphics/fonts/FontCrashDetectorTest.java
deleted file mode 100644
index 275e7c7fec04..000000000000
--- a/services/tests/servicestests/src/com/android/server/graphics/fonts/FontCrashDetectorTest.java
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.server.graphics.fonts;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import android.content.Context;
-import android.os.FileUtils;
-import android.platform.test.annotations.Presubmit;
-
-import androidx.test.InstrumentationRegistry;
-import androidx.test.filters.SmallTest;
-import androidx.test.runner.AndroidJUnit4;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-import java.io.File;
-
-@Presubmit
-@SmallTest
-@RunWith(AndroidJUnit4.class)
-public final class FontCrashDetectorTest {
-
-    private File mCacheDir;
-
-    @SuppressWarnings("ResultOfMethodCallIgnored")
-    @Before
-    public void setUp() {
-        Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
-        mCacheDir = new File(context.getCacheDir(), "UpdatableFontDirTest");
-        FileUtils.deleteContentsAndDir(mCacheDir);
-        mCacheDir.mkdirs();
-    }
-
-    @Test
-    public void detectCrash() throws Exception {
-        // Prepare a marker file.
-        File file = new File(mCacheDir, "detectCrash");
-        assertThat(file.createNewFile()).isTrue();
-
-        FontCrashDetector detector = new FontCrashDetector(file);
-        assertThat(detector.hasCrashed()).isTrue();
-
-        detector.clear();
-        assertThat(detector.hasCrashed()).isFalse();
-        assertThat(file.exists()).isFalse();
-    }
-
-    @Test
-    public void monitorCrash() {
-        File file = new File(mCacheDir, "monitorCrash");
-        FontCrashDetector detector = new FontCrashDetector(file);
-        assertThat(detector.hasCrashed()).isFalse();
-
-        FontCrashDetector.MonitoredBlock block = detector.start();
-        assertThat(file.exists()).isTrue();
-
-        block.close();
-        assertThat(file.exists()).isFalse();
-    }
-}
diff --git a/tests/UpdatableSystemFontTest/Android.bp b/tests/UpdatableSystemFontTest/Android.bp
index ee24d48f0ed5..d4f1ad317d31 100644
--- a/tests/UpdatableSystemFontTest/Android.bp
+++ b/tests/UpdatableSystemFontTest/Android.bp
@@ -26,13 +26,9 @@ java_test_host {
     srcs: ["src/**/*.java"],
     libs: ["tradefed", "compatibility-tradefed", "compatibility-host-util"],
     static_libs: [
-        "block_device_writer_jar",
         "frameworks-base-hostutils",
     ],
     test_suites: ["general-tests", "vts"],
-    target_required: [
-        "block_device_writer_module",
-    ],
     data: [
         ":NotoColorEmojiTtf",
         ":UpdatableSystemFontTestCertDer",
diff --git a/tests/UpdatableSystemFontTest/AndroidTest.xml b/tests/UpdatableSystemFontTest/AndroidTest.xml
index 7b919bd4b114..efe5d703880c 100644
--- a/tests/UpdatableSystemFontTest/AndroidTest.xml
+++ b/tests/UpdatableSystemFontTest/AndroidTest.xml
@@ -21,7 +21,6 @@
 
     <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
         <option name="cleanup" value="true" />
-        <option name="push" value="block_device_writer->/data/local/tmp/block_device_writer" />
         <option name="push" value="UpdatableSystemFontTestCert.der->/data/local/tmp/UpdatableSystemFontTestCert.der" />
         <option name="push" value="NotoColorEmoji.ttf->/data/local/tmp/NotoColorEmoji.ttf" />
         <option name="push" value="UpdatableSystemFontTestNotoColorEmoji.ttf.fsv_sig->/data/local/tmp/UpdatableSystemFontTestNotoColorEmoji.ttf.fsv_sig" />
diff --git a/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java b/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java
index e249f8a99b0c..92fa498f8326 100644
--- a/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java
+++ b/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java
@@ -21,7 +21,6 @@ import static com.google.common.truth.Truth.assertWithMessage;
 
 import android.platform.test.annotations.RootPermissionTest;
 
-import com.android.blockdevicewriter.BlockDeviceWriter;
 import com.android.fsverity.AddFsVerityCertRule;
 import com.android.tradefed.device.DeviceNotAvailableException;
 import com.android.tradefed.log.LogUtil.CLog;
@@ -144,30 +143,6 @@ public class UpdatableSystemFontTest extends BaseHostJUnit4Test {
         assertThat(fontPathAfterReboot).isEqualTo(fontPath);
     }
 
-    @Test
-    public void reboot_clearDamagedFiles() throws Exception {
-        expectRemoteCommandToSucceed(String.format("cmd font update %s %s",
-                TEST_NOTO_COLOR_EMOJI_V1_TTF, TEST_NOTO_COLOR_EMOJI_V1_TTF_FSV_SIG));
-        String fontPath = getFontPath(NOTO_COLOR_EMOJI_TTF);
-        assertThat(fontPath).startsWith("/data/fonts/files/");
-        assertThat(BlockDeviceWriter.canReadByte(getDevice(), fontPath, 0)).isTrue();
-
-        BlockDeviceWriter.damageFileAgainstBlockDevice(getDevice(), fontPath, 0);
-        expectRemoteCommandToSucceed("stop");
-        // We have to make sure system_server is gone before dropping caches, because system_server
-        // process holds font memory maps and prevents cache eviction.
-        waitUntilSystemServerIsGone();
-        BlockDeviceWriter.assertFileNotOpen(getDevice(), fontPath);
-        BlockDeviceWriter.dropCaches(getDevice());
-        assertThat(BlockDeviceWriter.canReadByte(getDevice(), fontPath, 0)).isFalse();
-
-        expectRemoteCommandToSucceed("start");
-        waitUntilFontCommandIsReady();
-        String fontPathAfterReboot = getFontPath(NOTO_COLOR_EMOJI_TTF);
-        assertWithMessage("Damaged file should be deleted")
-                .that(fontPathAfterReboot).startsWith("/system");
-    }
-
     private String getFontPath(String fontFileName) throws Exception {
         // TODO: add a dedicated command for testing.
         String lines = expectRemoteCommandToSucceed("cmd font dump");