Don't time out when fetching tether offload stats.

Currently, fetching tethering offload stats has a 1000ms timeout,
after which we return stats of zero. However, returning zero is
invalid and will cause various parts of the network stats
accounting code to complain that statistics are moving backwards,
causing at least a log.wtf, and possibly a crash.

This CL removes the timeout entirely.

An alternative would have been to keep the timeout and return
null if the stats fetch failed. However, this complicates the
code, and if the HAL is persistently slow, could cause no stats
to be counted, ever. Given the impact of such behaviour on users'
data plans it is likely better to block until the stats are
collected.

Bug: 29337859
Bug: 32163131
Test: builds
Test: OffloadControllerTest passes
Change-Id: I9c9413d757f44e87a51ea7ca82b657cc6aa0d4ba

1 files changed, 17 insertions, 29 deletions

diff --git a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
index 1a5ff778010c..ccbac31c82c3 100644
--- a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
+++ b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
@@ -46,7 +46,6 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -58,8 +57,6 @@ import java.util.concurrent.TimeUnit;
 public class OffloadController {
     private static final String TAG = OffloadController.class.getSimpleName();
 
-    private static final int STATS_FETCH_TIMEOUT_MS = 1000;
-
     private final Handler mHandler;
     private final OffloadHardwareInterface mHwInterface;
     private final ContentResolver mContentResolver;
@@ -177,33 +174,24 @@ public class OffloadController {
         @Override
         public NetworkStats getTetherStats() {
             NetworkStats stats = new NetworkStats(SystemClock.elapsedRealtime(), 0);
-            CountDownLatch latch = new CountDownLatch(1);
-
-            mHandler.post(() -> {
-                try {
-                    NetworkStats.Entry entry = new NetworkStats.Entry();
-                    entry.set = SET_DEFAULT;
-                    entry.tag = TAG_NONE;
-                    entry.uid = UID_TETHERING;
-
-                    updateStatsForCurrentUpstream();
-
-                    for (String iface : mForwardedStats.keySet()) {
-                        entry.iface = iface;
-                        entry.rxBytes = mForwardedStats.get(iface).rxBytes;
-                        entry.txBytes = mForwardedStats.get(iface).txBytes;
-                        stats.addValues(entry);
-                    }
-                } finally {
-                    latch.countDown();
-                }
-            });
 
-            try {
-                latch.await(STATS_FETCH_TIMEOUT_MS, TimeUnit.MILLISECONDS);
-            } catch (InterruptedException e) {
-                mLog.e("Tethering stats fetch timed out after " + STATS_FETCH_TIMEOUT_MS + "ms");
-            }
+            // We can't just post to mHandler because we are mostly (but not always) called by
+            // NetworkStatsService#performPollLocked, which is (currently) on the same thread as us.
+            mHandler.runWithScissors(() -> {
+                NetworkStats.Entry entry = new NetworkStats.Entry();
+                entry.set = SET_DEFAULT;
+                entry.tag = TAG_NONE;
+                entry.uid = UID_TETHERING;
+
+                updateStatsForCurrentUpstream();
+
+                for (String iface : mForwardedStats.keySet()) {
+                    entry.iface = iface;
+                    entry.rxBytes = mForwardedStats.get(iface).rxBytes;
+                    entry.txBytes = mForwardedStats.get(iface).txBytes;
+                    stats.addValues(entry);
+                }
+            }, 0);
 
             return stats;
         }