commit a61c36281278e282ac53f5ba91b73748374e192f
author Chenbo Feng <fengc@google.com> Wed Apr 10 12:55:07 2019 -0700
committer Chenbo Feng <fengc@google.com> Wed Apr 10 12:59:18 2019 -0700
tree 792e931ef0da9690fecbffeefdb30bf2b3d21898
parent 54496af119a2c34f953cb38870483e2099a6c90e

Add a lock to protect persistent stats snapshot

Since the network stats could be polled from multiple services at
runtime, it is not thread safe for networkStatsFactory to hold a
persistent stats snapshot without any protection. Use a internal lock to
prevent concurrent modification on mPersistentSnapshot to fix the
problem.

Bug: 124764595
Test: android.app.usage.cts.NetworkUsageStatsTest
      android.net.cts.TrafficStatsTest

Change-Id: I22afb46f17697e8b6359d4f593802e0f4b95db8b

services/core/java/com/android/server/net/NetworkStatsFactory.java

diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java
index bf34d8f..bd11d46 100644
--- a/services/core/java/com/android/server/net/NetworkStatsFactory.java
+++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java
@@ -28,6 +28,7 @@
 import android.os.StrictMode;
 import android.os.SystemClock;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.ArrayUtils;
 import com.android.internal.util.ProcFileReader;
@@ -65,6 +66,7 @@
     private boolean mUseBpfStats;
 
     // A persistent Snapshot since device start for eBPF stats
+    @GuardedBy("mPersistSnapshot")
     private final NetworkStats mPersistSnapshot;
 
     // TODO: only do adjustments in NetworkStatsService and remove this.
@@ -289,15 +291,17 @@
                 stats = new NetworkStats(SystemClock.elapsedRealtime(), -1);
             }
             if (mUseBpfStats) {
-                if (nativeReadNetworkStatsDetail(stats, mStatsXtUid.getAbsolutePath(), UID_ALL,
-                        null, TAG_ALL, mUseBpfStats) != 0) {
-                    throw new IOException("Failed to parse network stats");
+                synchronized (mPersistSnapshot) {
+                    if (nativeReadNetworkStatsDetail(stats, mStatsXtUid.getAbsolutePath(), UID_ALL,
+                            null, TAG_ALL, mUseBpfStats) != 0) {
+                        throw new IOException("Failed to parse network stats");
+                    }
+                    mPersistSnapshot.setElapsedRealtime(stats.getElapsedRealtime());
+                    mPersistSnapshot.combineAllValues(stats);
+                    NetworkStats result = mPersistSnapshot.clone();
+                    result.filter(limitUid, limitIfaces, limitTag);
+                    return result;
                 }
-                mPersistSnapshot.setElapsedRealtime(stats.getElapsedRealtime());
-                mPersistSnapshot.combineAllValues(stats);
-                NetworkStats result = mPersistSnapshot.clone();
-                result.filter(limitUid, limitIfaces, limitTag);
-                return result;
             } else {
                 if (nativeReadNetworkStatsDetail(stats, mStatsXtUid.getAbsolutePath(), limitUid,
                         limitIfaces, limitTag, mUseBpfStats) != 0) {