From 5f729e14a325d7783a1d9d948abbdac0800f7233 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Sat, 17 Apr 2021 23:37:19 +0900 Subject: [PATCH] Make Tethering#dump asynchronous. This is in preparation for removing mPublicSync. Test: m Test: adb shell dumpsys tethering Test: atest TetheringTests TetheringIntegrationTests Change-Id: Ib2c9d0bff23614f76c8e075d32cb03412d3d21f7 --- .../tethering/BpfCoordinator.java | 63 ++++++++----------- .../tethering/EntitlementManager.java | 30 ++++----- .../networkstack/tethering/Tethering.java | 51 ++++++++++++--- 3 files changed, 81 insertions(+), 63 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index add4f37367..2eed968d51 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -47,7 +47,6 @@ import android.net.netstats.provider.NetworkStatsProvider; import android.net.util.InterfaceParams; import android.net.util.SharedLog; import android.net.util.TetheringUtils.ForwardedStats; -import android.os.ConditionVariable; import android.os.Handler; import android.system.ErrnoException; import android.text.TextUtils; @@ -735,43 +734,35 @@ public class BpfCoordinator { * be allowed to be accessed on the handler thread. */ public void dump(@NonNull IndentingPrintWriter pw) { - final ConditionVariable dumpDone = new ConditionVariable(); - mHandler.post(() -> { - pw.println("mIsBpfEnabled: " + mIsBpfEnabled); - pw.println("Polling " + (mPollingStarted ? "started" : "not started")); - pw.println("Stats provider " + (mStatsProvider != null - ? "registered" : "not registered")); - pw.println("Upstream quota: " + mInterfaceQuotas.toString()); - pw.println("Polling interval: " + getPollingInterval() + " ms"); - pw.println("Bpf shim: " + mBpfCoordinatorShim.toString()); + pw.println("mIsBpfEnabled: " + mIsBpfEnabled); + pw.println("Polling " + (mPollingStarted ? "started" : "not started")); + pw.println("Stats provider " + (mStatsProvider != null + ? "registered" : "not registered")); + pw.println("Upstream quota: " + mInterfaceQuotas.toString()); + pw.println("Polling interval: " + getPollingInterval() + " ms"); + pw.println("Bpf shim: " + mBpfCoordinatorShim.toString()); - pw.println("Forwarding stats:"); - pw.increaseIndent(); - if (mStats.size() == 0) { - pw.println(""); - } else { - dumpStats(pw); - } - pw.decreaseIndent(); - - pw.println("Forwarding rules:"); - pw.increaseIndent(); - dumpIpv6UpstreamRules(pw); - dumpIpv6ForwardingRules(pw); - dumpIpv4ForwardingRules(pw); - pw.decreaseIndent(); - - pw.println(); - pw.println("Forwarding counters:"); - pw.increaseIndent(); - dumpCounters(pw); - pw.decreaseIndent(); - - dumpDone.open(); - }); - if (!dumpDone.block(DUMP_TIMEOUT_MS)) { - pw.println("... dump timed-out after " + DUMP_TIMEOUT_MS + "ms"); + pw.println("Forwarding stats:"); + pw.increaseIndent(); + if (mStats.size() == 0) { + pw.println(""); + } else { + dumpStats(pw); } + pw.decreaseIndent(); + + pw.println("Forwarding rules:"); + pw.increaseIndent(); + dumpIpv6UpstreamRules(pw); + dumpIpv6ForwardingRules(pw); + dumpIpv4ForwardingRules(pw); + pw.decreaseIndent(); + + pw.println(); + pw.println("Forwarding counters:"); + pw.increaseIndent(); + dumpCounters(pw); + pw.decreaseIndent(); } private void dumpStats(@NonNull IndentingPrintWriter pw) { diff --git a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java index b1e3cfed52..60fcfd0437 100644 --- a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java +++ b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java @@ -41,7 +41,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.net.util.SharedLog; import android.os.Bundle; -import android.os.ConditionVariable; import android.os.Handler; import android.os.Parcel; import android.os.PersistableBundle; @@ -516,25 +515,18 @@ public class EntitlementManager { * @param pw {@link PrintWriter} is used to print formatted */ public void dump(PrintWriter pw) { - final ConditionVariable mWaiting = new ConditionVariable(); - mHandler.post(() -> { - pw.print("isCellularUpstreamPermitted: "); - pw.println(isCellularUpstreamPermitted()); - for (int type = mCurrentDownstreams.nextSetBit(0); type >= 0; - type = mCurrentDownstreams.nextSetBit(type + 1)) { - pw.print("Type: "); - pw.print(typeString(type)); - if (mCurrentEntitlementResults.indexOfKey(type) > -1) { - pw.print(", Value: "); - pw.println(errorString(mCurrentEntitlementResults.get(type))); - } else { - pw.println(", Value: empty"); - } + pw.print("isCellularUpstreamPermitted: "); + pw.println(isCellularUpstreamPermitted()); + for (int type = mCurrentDownstreams.nextSetBit(0); type >= 0; + type = mCurrentDownstreams.nextSetBit(type + 1)) { + pw.print("Type: "); + pw.print(typeString(type)); + if (mCurrentEntitlementResults.indexOfKey(type) > -1) { + pw.print(", Value: "); + pw.println(errorString(mCurrentEntitlementResults.get(type))); + } else { + pw.println(", Value: empty"); } - mWaiting.open(); - }); - if (!mWaiting.block(DUMP_TIMEOUT)) { - pw.println("... dump timed out after " + DUMP_TIMEOUT + "ms"); } pw.print("Exempted: ["); for (int type = mExemptedDownstreams.nextSetBit(0); type >= 0; diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 8db4cb99be..d2e726e023 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -147,8 +147,11 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Set; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; /** * @@ -166,6 +169,9 @@ public class Tethering { }; private static final SparseArray sMagicDecoderRing = MessageUtils.findMessageNames(sMessageClasses); + + private static final int DUMP_TIMEOUT_MS = 10_000; + // Keep in sync with NETID_UNSET in system/netd/include/netid_client.h private static final int NETID_UNSET = 0; @@ -2278,15 +2284,10 @@ public class Tethering { pw.decreaseIndent(); } - void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter writer, @Nullable String[] args) { + void doDump(@NonNull FileDescriptor fd, @NonNull PrintWriter writer, @Nullable String[] args) { // Binder.java closes the resource for us. - @SuppressWarnings("resource") - final IndentingPrintWriter pw = new IndentingPrintWriter(writer, " "); - if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) - != PERMISSION_GRANTED) { - pw.println("Permission Denial: can't dump."); - return; - } + @SuppressWarnings("resource") final IndentingPrintWriter pw = new IndentingPrintWriter( + writer, " "); if (argsContain(args, "bpf")) { dumpBpf(pw); @@ -2363,6 +2364,40 @@ public class Tethering { pw.decreaseIndent(); } + void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter writer, @Nullable String[] args) { + if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) + != PERMISSION_GRANTED) { + writer.println("Permission Denial: can't dump."); + return; + } + + final CountDownLatch latch = new CountDownLatch(1); + + // Don't crash the system if something in doDump throws an exception, but try to propagate + // the exception to the caller. + AtomicReference exceptionRef = new AtomicReference<>(); + mHandler.post(() -> { + try { + doDump(fd, writer, args); + } catch (RuntimeException e) { + exceptionRef.set(e); + } + latch.countDown(); + }); + + try { + if (!latch.await(DUMP_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + writer.println("Dump timeout after " + DUMP_TIMEOUT_MS + "ms"); + return; + } + } catch (InterruptedException e) { + exceptionRef.compareAndSet(null, new IllegalStateException("Dump interrupted", e)); + } + + final RuntimeException e = exceptionRef.get(); + if (e != null) throw e; + } + private static boolean argsContain(String[] args, String target) { for (String arg : args) { if (target.equals(arg)) return true;