Merge "Move dumpsys to handler thread" into main

service/src/com/android/server/ConnectivityService.java

@@ -303,6 +303,7 @@ import com.android.server.connectivity.DnsManager;
 import com.android.server.connectivity.DnsManager.PrivateDnsValidationUpdate;
 import com.android.server.connectivity.DscpPolicyTracker;
 import com.android.server.connectivity.FullScore;
+import com.android.server.connectivity.HandlerUtils;
 import com.android.server.connectivity.InvalidTagException;
 import com.android.server.connectivity.KeepaliveResourceUtil;
 import com.android.server.connectivity.KeepaliveTracker;
@@ -1255,16 +1256,24 @@ public class ConnectivityService extends IConnectivityManager.Stub
         private static final String PRIORITY_ARG = "--dump-priority";
         private static final String PRIORITY_ARG_HIGH = "HIGH";
         private static final String PRIORITY_ARG_NORMAL = "NORMAL";
+        private static final int DUMPSYS_DEFAULT_TIMEOUT_MS = 10_000;
         LocalPriorityDump() {}
         private void dumpHigh(FileDescriptor fd, PrintWriter pw) {
-            doDump(fd, pw, new String[] {DIAG_ARG});
-            doDump(fd, pw, new String[] {SHORT_ARG});
+            if (!HandlerUtils.runWithScissors(mHandler, () -> {
+                doDump(fd, pw, new String[]{DIAG_ARG});
+                doDump(fd, pw, new String[]{SHORT_ARG});
+            }, DUMPSYS_DEFAULT_TIMEOUT_MS)) {
+                pw.println("dumpHigh timeout");
+            }
         }
         private void dumpNormal(FileDescriptor fd, PrintWriter pw, String[] args) {
-            doDump(fd, pw, args);
+            if (!HandlerUtils.runWithScissors(mHandler, () -> doDump(fd, pw, args),
+                    DUMPSYS_DEFAULT_TIMEOUT_MS)) {
+                pw.println("dumpNormal timeout");
+            }
         }
         public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {

service/src/com/android/server/connectivity/HandlerUtils.java

+/*
+ * Copyright (C) 2023 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.connectivity;
+
+import android.annotation.NonNull;
+import android.os.Handler;
+import android.os.Looper;
+import android.os.SystemClock;
+
+/**
+ * Helper class for Handler related utilities.
+ *
+ * @hide
+ */
+public class HandlerUtils {
+    // Note: @hide methods copied from android.os.Handler
+    /**
+     * Runs the specified task synchronously.
+     * <p>
+     * If the current thread is the same as the handler thread, then the runnable
+     * runs immediately without being enqueued.  Otherwise, posts the runnable
+     * to the handler and waits for it to complete before returning.
+     * </p><p>
+     * This method is dangerous!  Improper use can result in deadlocks.
+     * Never call this method while any locks are held or use it in a
+     * possibly re-entrant manner.
+     * </p><p>
+     * This method is occasionally useful in situations where a background thread
+     * must synchronously await completion of a task that must run on the
+     * handler's thread.  However, this problem is often a symptom of bad design.
+     * Consider improving the design (if possible) before resorting to this method.
+     * </p><p>
+     * One example of where you might want to use this method is when you just
+     * set up a Handler thread and need to perform some initialization steps on
+     * it before continuing execution.
+     * </p><p>
+     * If timeout occurs then this method returns <code>false</code> but the runnable
+     * will remain posted on the handler and may already be in progress or
+     * complete at a later time.
+     * </p><p>
+     * When using this method, be sure to use {@link Looper#quitSafely} when
+     * quitting the looper.  Otherwise {@link #runWithScissors} may hang indefinitely.
+     * (TODO: We should fix this by making MessageQueue aware of blocking runnables.)
+     * </p>
+     *
+     * @param h The target handler.
+     * @param r The Runnable that will be executed synchronously.
+     * @param timeout The timeout in milliseconds, or 0 to wait indefinitely.
+     *
+     * @return Returns true if the Runnable was successfully executed.
+     *         Returns false on failure, usually because the
+     *         looper processing the message queue is exiting.
+     *
+     * @hide This method is prone to abuse and should probably not be in the API.
+     * If we ever do make it part of the API, we might want to rename it to something
+     * less funny like runUnsafe().
+     */
+    public static boolean runWithScissors(@NonNull Handler h, @NonNull Runnable r, long timeout) {
+        if (r == null) {
+            throw new IllegalArgumentException("runnable must not be null");
+        }
+        if (timeout < 0) {
+            throw new IllegalArgumentException("timeout must be non-negative");
+        }
+
+        if (Looper.myLooper() == h.getLooper()) {
+            r.run();
+            return true;
+        }
+
+        BlockingRunnable br = new BlockingRunnable(r);
+        return br.postAndWait(h, timeout);
+    }
+
+    private static final class BlockingRunnable implements Runnable {
+        private final Runnable mTask;
+        private boolean mDone;
+
+        BlockingRunnable(Runnable task) {
+            mTask = task;
+        }
+
+        @Override
+        public void run() {
+            try {
+                mTask.run();
+            } finally {
+                synchronized (this) {
+                    mDone = true;
+                    notifyAll();
+                }
+            }
+        }
+
+        public boolean postAndWait(Handler handler, long timeout) {
+            if (!handler.post(this)) {
+                return false;
+            }
+
+            synchronized (this) {
+                if (timeout > 0) {
+                    final long expirationTime = SystemClock.uptimeMillis() + timeout;
+                    while (!mDone) {
+                        long delay = expirationTime - SystemClock.uptimeMillis();
+                        if (delay <= 0) {
+                            return false; // timeout
+                        }
+                        try {
+                            wait(delay);
+                        } catch (InterruptedException ex) {
+                        }
+                    }
+                } else {
+                    while (!mDone) {
+                        try {
+                            wait();
+                        } catch (InterruptedException ex) {
+                        }
+                    }
+                }
+            }
+            return true;
+        }
+    }
+}

tests/unit/java/com/android/server/HandlerUtilsTest.kt

+/*
+ * Copyright (C) 2023 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
+
+import android.os.HandlerThread
+import com.android.server.connectivity.HandlerUtils
+import com.android.testutils.DevSdkIgnoreRunner
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+
+const val THREAD_BLOCK_TIMEOUT_MS = 1000L
+const val TEST_REPEAT_COUNT = 100
+@RunWith(DevSdkIgnoreRunner::class)
+class HandlerUtilsTest {
+    val handlerThread = HandlerThread("HandlerUtilsTestHandlerThread").also {
+        it.start()
+    }
+    val handler = handlerThread.threadHandler
+
+    @Test
+    fun testRunWithScissors() {
+        // Repeat the test a fair amount of times to ensure that it does not pass by chance.
+        repeat(TEST_REPEAT_COUNT) {
+            var result = false
+            HandlerUtils.runWithScissors(handler, {
+                assertEquals(Thread.currentThread(), handlerThread)
+                result = true
+            }, THREAD_BLOCK_TIMEOUT_MS)
+            // Assert that the result is modified on the handler thread, but can also be seen from
+            // the current thread. The assertion should pass if the runWithScissors provides
+            // the guarantee where the assignment happens-before the assertion.
+            assertTrue(result)
+        }
+    }
+
+    @After
+    fun tearDown() {
+        handlerThread.quitSafely()
+        handlerThread.join()
+    }
+}