From 3a7f671f452dd2537dafbd31022e1d3c2f355819 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 14 Jul 2021 06:44:37 +0000 Subject: [PATCH 1/2] Fix mMobileIfaces is not protected by lock Currently, mMobileIfaces is accessed from multiple threads, and should be protected from concurrent accessing. However, since the variable could be accessed frequently, holding the mStatsLock would make this be blocked by network stats I/O operations. Thus, protect the variable by making it volatile. Test: Wifi on/off stress test Bug: 192758557 Change-Id: Ie7694a63f5203ee7c83830ca13d97219b7949fd7 --- .../com/android/server/net/NetworkStatsService.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 4ee867b7d0..f571b35f8c 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -289,8 +289,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private String mActiveIface; /** Set of any ifaces associated with mobile networks since boot. */ - @GuardedBy("mStatsLock") - private String[] mMobileIfaces = new String[0]; + private volatile String[] mMobileIfaces = new String[0]; /** Set of all ifaces currently used by traffic that does not explicitly specify a Network. */ @GuardedBy("mStatsLock") @@ -935,7 +934,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @Override public String[] getMobileIfaces() { - return mMobileIfaces; + return mMobileIfaces.clone(); } @Override @@ -1084,7 +1083,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } @Override - public long getIfaceStats(String iface, int type) { + public long getIfaceStats(@NonNull String iface, int type) { + Objects.requireNonNull(iface); long nativeIfaceStats = nativeGetIfaceStat(iface, type, checkBpfStatsEnable()); if (nativeIfaceStats == -1) { return nativeIfaceStats; @@ -1382,7 +1382,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } - mMobileIfaces = mobileIfaces.toArray(new String[mobileIfaces.size()]); + mMobileIfaces = mobileIfaces.toArray(new String[0]); } private static int getSubIdForMobile(@NonNull NetworkStateSnapshot state) { From 73993fbb6815a2eeeb7f78f551070c19b17bb538 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Mon, 26 Jul 2021 09:16:59 +0000 Subject: [PATCH 2/2] Add debug log for tracking NPE of mMobileIfaces Test: TH Bug: 192758557 Change-Id: Ib048c18b1c64627de5a9d2b04d10e084a014ff64 --- .../com/android/server/net/NetworkStatsService.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index f571b35f8c..097b0711ef 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -934,6 +934,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @Override public String[] getMobileIfaces() { + // TODO (b/192758557): Remove debug log. + if (ArrayUtils.contains(mMobileIfaces, null)) { + throw new NullPointerException( + "null element in mMobileIfaces: " + Arrays.toString(mMobileIfaces)); + } return mMobileIfaces.clone(); } @@ -1383,6 +1388,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } mMobileIfaces = mobileIfaces.toArray(new String[0]); + // TODO (b/192758557): Remove debug log. + if (ArrayUtils.contains(mMobileIfaces, null)) { + throw new NullPointerException( + "null element in mMobileIfaces: " + Arrays.toString(mMobileIfaces)); + } } private static int getSubIdForMobile(@NonNull NetworkStateSnapshot state) {