Close FDs / mmap handles promptly.

Currently we rely on GC to close them.
This may cause system_server crash depending on the timing of GC.

Bug: 187879195
Test: atest UpdatableSystemFontTest
Change-Id: I09ac3f349e5ec100e4164320cbf27977474cc4bb

1 files changed, 67 insertions, 0 deletions

diff --git a/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java b/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java
index 898b8d4cbdc1..44f96c5a987b 100644
--- a/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java
+++ b/tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java
@@ -56,6 +56,11 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.regex.Pattern;
 
 /**
  * Tests if fonts can be updated by {@link FontManager} API.
@@ -95,6 +100,12 @@ public class UpdatableSystemFontTest {
             EMOJI_RENDERING_TEST_APP_ID + "/.EmojiRenderingTestActivity";
     private static final long ACTIVITY_TIMEOUT_MILLIS = SECONDS.toMillis(10);
 
+    private static final Pattern PATTERN_FONT_FILES = Pattern.compile("\\.(ttf|otf|ttc|otc)$");
+    private static final Pattern PATTERN_TMP_FILES = Pattern.compile("^/data/local/tmp/");
+    private static final Pattern PATTERN_DATA_FONT_FILES = Pattern.compile("^/data/fonts/files/");
+    private static final Pattern PATTERN_SYSTEM_FONT_FILES =
+            Pattern.compile("^/(system|product)/fonts/");
+
     private String mKeyId;
     private FontManager mFontManager;
 
@@ -236,6 +247,32 @@ public class UpdatableSystemFontTest {
         assertThat(fontPathAfterReboot).isEqualTo(fontPath);
     }
 
+
+    @Test
+    public void fdLeakTest() throws Exception {
+        long originalOpenFontCount =
+                countMatch(getOpenFiles("system_server"), PATTERN_FONT_FILES);
+        Pattern patternEmojiVPlus1 =
+                Pattern.compile(Pattern.quote(TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF));
+        for (int i = 0; i < 10; i++) {
+            assertThat(updateFontFile(
+                    TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF, TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF_FSV_SIG))
+                    .isEqualTo(FontManager.RESULT_SUCCESS);
+            List<String> openFiles = getOpenFiles("system_server");
+            for (Pattern p : Arrays.asList(PATTERN_FONT_FILES, PATTERN_SYSTEM_FONT_FILES,
+                    PATTERN_DATA_FONT_FILES, PATTERN_TMP_FILES)) {
+                Log.i(TAG, String.format("num of %s: %d", p, countMatch(openFiles, p)));
+            }
+            // system_server should not keep /data/fonts files open.
+            assertThat(countMatch(openFiles, PATTERN_DATA_FONT_FILES)).isEqualTo(0);
+            // system_server should not keep passed FD open.
+            assertThat(countMatch(openFiles, patternEmojiVPlus1)).isEqualTo(0);
+            // The number of open font FD should not increase.
+            assertThat(countMatch(openFiles, PATTERN_FONT_FILES))
+                    .isAtMost(originalOpenFontCount);
+        }
+    }
+
     private static String insertCert(String certPath) throws Exception {
         Pair<String, String> result;
         try (InputStream is = new FileInputStream(certPath)) {
@@ -338,7 +375,37 @@ public class UpdatableSystemFontTest {
         return !expectCommandToSucceed(cmd).trim().isEmpty();
     }
 
+    private static List<String> getOpenFiles(String appId) throws Exception {
+        String pid = pidOf(appId);
+        if (pid.isEmpty()) {
+            return Collections.emptyList();
+        }
+        String cmd = String.format("lsof -p %s", pid);
+        String out = expectCommandToSucceed(cmd);
+        List<String> paths = new ArrayList<>();
+        boolean first = true;
+        for (String line : out.split("\n")) {
+            // Skip the header.
+            if (first) {
+                first = false;
+                continue;
+            }
+            String[] records = line.split(" ");
+            if (records.length > 0) {
+                paths.add(records[records.length - 1]);
+            }
+        }
+        return paths;
+    }
+
     private static String pidOf(String appId) throws Exception {
         return expectCommandToSucceed("pidof " + appId).trim();
     }
+
+    private static long countMatch(List<String> paths, Pattern pattern) {
+        // Note: asPredicate() returns true for partial matching.
+        return paths.stream()
+                .filter(pattern.asPredicate())
+                .count();
+    }
 }