Merge "Freezer: fix exception when parsing /proc/locks" into sc-dev

core/java/android/os/Process.java

@@ -34,12 +34,9 @@ import dalvik.system.VMRuntime;
 
 import libcore.io.IoUtils;
 
-import java.io.BufferedReader;
 import java.io.FileDescriptor;
-import java.io.FileReader;
 import java.io.IOException;
 import java.util.Map;
-import java.util.StringTokenizer;
 import java.util.concurrent.TimeoutException;
 
 /**
@@ -1472,43 +1469,4 @@ public class Process {
     }
 
     private static native int nativePidFdOpen(int pid, int flags) throws ErrnoException;
-
-    /**
-     * Checks if a process corresponding to a specific pid owns any file locks.
-     * @param pid The process ID for which we want to know the existence of file locks.
-     * @return true If the process holds any file locks, false otherwise.
-     * @throws IOException if /proc/locks can't be accessed.
-     *
-     * @hide
-     */
-    public static boolean hasFileLocks(int pid) throws Exception {
-        BufferedReader br = null;
-
-        try {
-            br = new BufferedReader(new FileReader("/proc/locks"));
-            String line;
-
-            while ((line = br.readLine()) != null) {
-                StringTokenizer st = new StringTokenizer(line);
-
-                for (int i = 0; i < 5 && st.hasMoreTokens(); i++) {
-                    String str = st.nextToken();
-                    try {
-                        if (i == 4 && Integer.parseInt(str) == pid) {
-                            return true;
-                        }
-                    } catch (NumberFormatException nfe) {
-                        throw new Exception("Exception parsing /proc/locks at \" "
-                                + line +  " \", token #" + i);
-                    }
-                }
-            }
-
-            return false;
-        } finally {
-            if (br != null) {
-                br.close();
-            }
-        }
-    }
 }

core/java/com/android/internal/os/ProcLocksReader.java

@@ -0,0 +1,89 @@
+/*
+ * 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.internal.os;
+
+import com.android.internal.util.ProcFileReader;
+
+import libcore.io.IoUtils;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+
+/**
+ * Reads and parses {@code locks} files in the {@code proc} filesystem.
+ * A typical example of /proc/locks
+ *
+ * 1: POSIX  ADVISORY  READ  18403 fd:09:9070 1073741826 1073742335
+ * 2: POSIX  ADVISORY  WRITE 18292 fd:09:34062 0 EOF
+ * 2: -> POSIX  ADVISORY  WRITE 18291 fd:09:34062 0 EOF
+ * 2: -> POSIX  ADVISORY  WRITE 18293 fd:09:34062 0 EOF
+ * 3: POSIX  ADVISORY  READ  3888 fd:09:13992 128 128
+ * 4: POSIX  ADVISORY  READ  3888 fd:09:14230 1073741826 1073742335
+ */
+public class ProcLocksReader {
+    private final String mPath;
+
+    public ProcLocksReader() {
+        mPath = "/proc/locks";
+    }
+
+    public ProcLocksReader(String path) {
+        mPath = path;
+    }
+
+    /**
+     * Checks if a process corresponding to a specific pid owns any file locks.
+     * @param pid The process ID for which we want to know the existence of file locks.
+     * @return true If the process holds any file locks, false otherwise.
+     * @throws IOException if /proc/locks can't be accessed.
+     */
+    public boolean hasFileLocks(int pid) throws Exception {
+        ProcFileReader reader = null;
+        long last = -1;
+        long id; // ordinal position of the lock in the list
+        int owner; // the PID of the process that owns the lock
+
+        try {
+            reader = new ProcFileReader(new FileInputStream(mPath));
+
+            while (reader.hasMoreData()) {
+                id = reader.nextLong(true); // lock id
+                if (id == last) {
+                    reader.finishLine(); // blocked lock
+                    continue;
+                }
+
+                reader.nextIgnored(); // lock type: POSIX?
+                reader.nextIgnored(); // lock type: MANDATORY?
+                reader.nextIgnored(); // lock type: RW?
+
+                owner = reader.nextInt(); // pid
+                if (owner == pid) {
+                    return true;
+                }
+                reader.finishLine();
+                last = id;
+            }
+        } catch (IOException e) {
+            // TODO: let ProcFileReader log the failed line
+            throw new Exception("Exception parsing /proc/locks");
+        } finally {
+            IoUtils.closeQuietly(reader);
+        }
+        return false;
+    }
+}

core/java/com/android/internal/util/ProcFileReader.java

@@ -28,8 +28,8 @@ import java.nio.charset.StandardCharsets;
  * requires each line boundary to be explicitly acknowledged using
  * {@link #finishLine()}. Assumes {@link StandardCharsets#US_ASCII} encoding.
  * <p>
- * Currently doesn't support formats based on {@code \0}, tabs, or repeated
+ * Currently doesn't support formats based on {@code \0}, tabs.
- * delimiters.
+ * Consecutive spaces are treated as a single delimiter.
  */
 public class ProcFileReader implements Closeable {
     private final InputStream mStream;
@@ -75,6 +75,11 @@ public class ProcFileReader implements Closeable {
     private void consumeBuf(int count) throws IOException {
         // TODO: consider moving to read pointer, but for now traceview says
         // these copies aren't a bottleneck.
+
+        // skip all consecutive delimiters.
+        while (count < mTail && mBuffer[count] == ' ') {
+            count++;
+        }
         System.arraycopy(mBuffer, count, mBuffer, 0, mTail - count);
         mTail -= count;
         if (mTail == 0) {
@@ -159,11 +164,18 @@ public class ProcFileReader implements Closeable {
      * Parse and return next token as base-10 encoded {@code long}.
      */
     public long nextLong() throws IOException {
+        return nextLong(false);
+    }
+
+    /**
+     * Parse and return next token as base-10 encoded {@code long}.
+     */
+    public long nextLong(boolean stopAtInvalid) throws IOException {
         final int tokenIndex = nextTokenIndex();
         if (tokenIndex == -1) {
             throw new ProtocolException("Missing required long");
         } else {
-            return parseAndConsumeLong(tokenIndex);
+            return parseAndConsumeLong(tokenIndex, stopAtInvalid);
         }
     }
 
@@ -176,7 +188,7 @@ public class ProcFileReader implements Closeable {
         if (tokenIndex == -1) {
             return def;
         } else {
-            return parseAndConsumeLong(tokenIndex);
+            return parseAndConsumeLong(tokenIndex, false);
         }
     }
 
@@ -186,7 +198,10 @@ public class ProcFileReader implements Closeable {
         return s;
     }
 
-    private long parseAndConsumeLong(int tokenIndex) throws IOException {
+    /**
+     * If stopAtInvalid is true, don't throw IOException but return whatever parsed so far.
+     */
+    private long parseAndConsumeLong(int tokenIndex, boolean stopAtInvalid) throws IOException {
         final boolean negative = mBuffer[0] == '-';
 
         // TODO: refactor into something like IntegralToString
@@ -194,8 +209,12 @@ public class ProcFileReader implements Closeable {
         for (int i = negative ? 1 : 0; i < tokenIndex; i++) {
             final int digit = mBuffer[i] - '0';
             if (digit < 0 || digit > 9) {
+                if (stopAtInvalid) {
+                    break;
+                } else {
                     throw invalidLong(tokenIndex);
                 }
+            }
 
             // always parse as negative number and apply sign later; this
             // correctly handles MIN_VALUE which is "larger" than MAX_VALUE.
@@ -226,6 +245,18 @@ public class ProcFileReader implements Closeable {
         return (int) value;
     }
 
+    /**
+     * Bypass the next token.
+     */
+    public void nextIgnored() throws IOException {
+        final int tokenIndex = nextTokenIndex();
+        if (tokenIndex == -1) {
+            throw new ProtocolException("Missing required token");
+        } else {
+            consumeBuf(tokenIndex + 1);
+        }
+    }
+
     @Override
     public void close() throws IOException {
         mStream.close();

core/tests/coretests/src/com/android/internal/os/ProcLocksReaderTest.java

@@ -0,0 +1,90 @@
+/*
+ * 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.internal.os;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import android.content.Context;
+import android.os.FileUtils;
+
+import androidx.test.InstrumentationRegistry;
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.nio.file.Files;
+
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class ProcLocksReaderTest {
+    private File mProcDirectory;
+
+    @Before
+    public void setUp() {
+        Context context = InstrumentationRegistry.getContext();
+        mProcDirectory = context.getDir("proc", Context.MODE_PRIVATE);
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        FileUtils.deleteContents(mProcDirectory);
+    }
+
+    @Test
+    public void testRunSimpleLocks() throws Exception {
+        String simpleLocks =
+                "1: POSIX  ADVISORY  READ  18403 fd:09:9070 1073741826 1073742335\n" +
+                "2: POSIX  ADVISORY  WRITE 18292 fd:09:34062 0 EOF\n";
+        assertFalse(runHasFileLocks(simpleLocks, 18402));
+        assertFalse(runHasFileLocks(simpleLocks, 18404));
+        assertTrue(runHasFileLocks(simpleLocks, 18403));
+        assertTrue(runHasFileLocks(simpleLocks, 18292));
+    }
+
+    @Test
+    public void testRunBlockedLocks() throws Exception {
+        String blockedLocks =
+                "1: POSIX  ADVISORY  READ  18403 fd:09:9070 1073741826 1073742335\n" +
+                "2: POSIX  ADVISORY  WRITE 18292 fd:09:34062 0 EOF\n" +
+                "2: -> POSIX  ADVISORY  WRITE 18291 fd:09:34062 0 EOF\n" +
+                "2: -> POSIX  ADVISORY  WRITE 18293 fd:09:34062 0 EOF\n" +
+                "3: POSIX  ADVISORY  READ  3888 fd:09:13992 128 128\n" +
+                "4: POSIX  ADVISORY  READ  3888 fd:09:14230 1073741826 1073742335\n";
+        assertFalse(runHasFileLocks(blockedLocks, 18402));
+        assertFalse(runHasFileLocks(blockedLocks, 18404));
+        assertTrue(runHasFileLocks(blockedLocks, 18403));
+        assertTrue(runHasFileLocks(blockedLocks, 18292));
+
+        assertFalse(runHasFileLocks(blockedLocks, 18291));
+        assertFalse(runHasFileLocks(blockedLocks, 18293));
+        assertTrue(runHasFileLocks(blockedLocks, 3888));
+    }
+
+    private boolean runHasFileLocks(String fileContents, int pid) throws Exception {
+        File tempFile = File.createTempFile("locks", null, mProcDirectory);
+        Files.write(tempFile.toPath(), fileContents.getBytes());
+        boolean result = new ProcLocksReader(tempFile.toString()).hasFileLocks(pid);
+        Files.delete(tempFile.toPath());
+        return result;
+    }
+}

core/tests/utiltests/src/com/android/internal/util/ProcFileReaderTest.java

@@ -166,6 +166,46 @@ public class ProcFileReaderTest extends AndroidTestCase {
         assertEquals(-1L, reader.nextOptionalLong(-1L));
     }
 
+    public void testInvalidLongs() throws Exception {
+        final ProcFileReader reader = buildReader("12: 34\n56 78@#\n");
+
+        assertEquals(12L, reader.nextLong(true));
+        assertEquals(34L, reader.nextLong(true));
+        reader.finishLine();
+        assertTrue(reader.hasMoreData());
+
+        assertEquals(56L, reader.nextLong(true));
+        assertEquals(78L, reader.nextLong(true));
+        reader.finishLine();
+        assertFalse(reader.hasMoreData());
+    }
+
+    public void testConsecutiveDelimiters() throws Exception {
+        final ProcFileReader reader = buildReader("1 2  3   4     5\n");
+
+        assertEquals(1L, reader.nextLong());
+        assertEquals(2L, reader.nextLong());
+        assertEquals(3L, reader.nextLong());
+        assertEquals(4L, reader.nextLong());
+        assertEquals(5L, reader.nextLong());
+        reader.finishLine();
+        assertFalse(reader.hasMoreData());
+    }
+
+    public void testIgnore() throws Exception {
+        final ProcFileReader reader = buildReader("a b c\n");
+
+        assertEquals("a", reader.nextString());
+        assertTrue(reader.hasMoreData());
+
+        reader.nextIgnored();
+        assertTrue(reader.hasMoreData());
+
+        assertEquals("c", reader.nextString());
+        reader.finishLine();
+        assertFalse(reader.hasMoreData());
+    }
+
     private static ProcFileReader buildReader(String string) throws IOException {
         return buildReader(string, 2048);
     }

services/core/java/com/android/server/am/CachedAppOptimizer.java

@@ -41,6 +41,7 @@ import android.util.Slog;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.os.ProcLocksReader;
 import com.android.internal.util.FrameworkStatsLog;
 import com.android.server.ServiceThread;
 
@@ -319,6 +320,7 @@ public final class CachedAppOptimizer {
     private int mPersistentCompactionCount;
     private int mBfgsCompactionCount;
     private final ProcessDependencies mProcessDependencies;
+    private final ProcLocksReader mProcLocksReader;
 
     public CachedAppOptimizer(ActivityManagerService am) {
         this(am, null, new DefaultProcessDependencies());
@@ -335,6 +337,7 @@ public final class CachedAppOptimizer {
         mProcessDependencies = processDependencies;
         mTestCallback = callback;
         mSettingsObserver = new SettingsContentObserver();
+        mProcLocksReader = new ProcLocksReader();
     }
 
     /**
@@ -1312,7 +1315,7 @@ public final class CachedAppOptimizer {
 
             try {
                 // pre-check for locks to avoid unnecessary freeze/unfreeze operations
-                if (Process.hasFileLocks(pid)) {
+                if (mProcLocksReader.hasFileLocks(pid)) {
                     if (DEBUG_FREEZER) {
                         Slog.d(TAG_AM, name + " (" + pid + ") holds file locks, not freezing");
                     }
@@ -1399,7 +1402,7 @@ public final class CachedAppOptimizer {
 
             try {
                 // post-check to prevent races
-                if (Process.hasFileLocks(pid)) {
+                if (mProcLocksReader.hasFileLocks(pid)) {
                     if (DEBUG_FREEZER) {
                         Slog.d(TAG_AM, name + " (" + pid + ") holds file locks, reverting freeze");
                     }