Merge "NetworkStats: Fix race condition causing system server crashes"

am: 08b928e8ed

Change-Id: I76ffca2326dd9287021ce9b012fd9b78cfd0c72b
This commit is contained in:
Lorenzo Colitti
2019-06-25 01:30:01 -07:00
committed by android-build-merger
2 changed files with 10 additions and 73 deletions

View File

@@ -373,14 +373,9 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
public void systemReady() {
mSystemReady = true;
if (!isBandwidthControlEnabled()) {
Slog.w(TAG, "bandwidth controls disabled, unable to track stats");
return;
}
synchronized (mStatsLock) {
mSystemReady = true;
// create data recorders along with historical rotators
mDevRecorder = buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false);
mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false);
@@ -426,7 +421,14 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
// ignored; service lives in system_server
}
registerPollAlarmLocked();
// schedule periodic pall alarm based on {@link NetworkStatsSettings#getPollInterval()}.
final PendingIntent pollIntent =
PendingIntent.getBroadcast(mContext, 0, new Intent(ACTION_NETWORK_STATS_POLL), 0);
final long currentRealtime = SystemClock.elapsedRealtime();
mAlarmManager.setInexactRepeating(AlarmManager.ELAPSED_REALTIME, currentRealtime,
mSettings.getPollInterval(), pollIntent);
registerGlobalAlert();
}
@@ -486,23 +488,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
}
/**
* Clear any existing {@link #ACTION_NETWORK_STATS_POLL} alarms, and
* reschedule based on current {@link NetworkStatsSettings#getPollInterval()}.
*/
private void registerPollAlarmLocked() {
if (mPollIntent != null) {
mAlarmManager.cancel(mPollIntent);
}
mPollIntent = PendingIntent.getBroadcast(
mContext, 0, new Intent(ACTION_NETWORK_STATS_POLL), 0);
final long currentRealtime = SystemClock.elapsedRealtime();
mAlarmManager.setInexactRepeating(AlarmManager.ELAPSED_REALTIME, currentRealtime,
mSettings.getPollInterval(), mPollIntent);
}
/**
* Register for a global alert that is delivered through
* {@link INetworkManagementEventObserver} once a threshold amount of data
@@ -548,8 +533,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
private INetworkStatsSession openSessionInternal(final int flags, final String callingPackage) {
assertBandwidthControlEnabled();
final int callingUid = Binder.getCallingUid();
final int usedFlags = isRateLimitedForPoll(callingUid)
? flags & (~NetworkStatsManager.FLAG_POLL_ON_OPEN)
@@ -742,7 +725,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
private long getNetworkTotalBytes(NetworkTemplate template, long start, long end) {
assertSystemReady();
assertBandwidthControlEnabled();
// NOTE: if callers want to get non-augmented data, they should go
// through the public API
@@ -753,7 +735,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
private NetworkStats getNetworkUidBytes(NetworkTemplate template, long start, long end) {
assertSystemReady();
assertBandwidthControlEnabled();
final NetworkStatsCollection uidComplete;
synchronized (mStatsLock) {
@@ -768,7 +749,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
if (Binder.getCallingUid() != uid) {
mContext.enforceCallingOrSelfPermission(ACCESS_NETWORK_STATE, TAG);
}
assertBandwidthControlEnabled();
// TODO: switch to data layer stats once kernel exports
// for now, read network layer stats and flatten across all ifaces
@@ -855,7 +835,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
NetworkState[] networkStates,
String activeIface) {
checkNetworkStackPermission(mContext);
assertBandwidthControlEnabled();
final long token = Binder.clearCallingIdentity();
try {
@@ -868,7 +847,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@Override
public void forceUpdate() {
mContext.enforceCallingOrSelfPermission(READ_NETWORK_USAGE_HISTORY, TAG);
assertBandwidthControlEnabled();
final long token = Binder.clearCallingIdentity();
try {
@@ -879,8 +857,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
private void advisePersistThreshold(long thresholdBytes) {
assertBandwidthControlEnabled();
// clamp threshold into safe range
mPersistThreshold = MathUtils.constrain(thresholdBytes, 128 * KB_IN_BYTES, 2 * MB_IN_BYTES);
if (LOGV) {
@@ -1739,24 +1715,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
}
private void assertBandwidthControlEnabled() {
if (!isBandwidthControlEnabled()) {
throw new IllegalStateException("Bandwidth module disabled");
}
}
private boolean isBandwidthControlEnabled() {
final long token = Binder.clearCallingIdentity();
try {
return mNetworkManager.isBandwidthControlEnabled();
} catch (RemoteException e) {
// ignored; service lives in system_server
return false;
} finally {
Binder.restoreCallingIdentity(token);
}
}
private class DropBoxNonMonotonicObserver implements NonMonotonicObserver<String> {
@Override
public void foundNonMonotonic(NetworkStats left, int leftIndex, NetworkStats right,

View File

@@ -239,7 +239,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -283,7 +282,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -357,7 +355,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -398,7 +395,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
@@ -433,7 +429,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
.addValues(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1536L, 12L, 512L, 4L, 0L)
.addValues(TEST_IFACE, UID_RED, SET_DEFAULT, 0xF00D, 512L, 4L, 512L, 4L, 0L)
.addValues(TEST_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 512L, 4L, 0L, 0L, 0L));
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
forcePollAndWaitForIdle();
@@ -473,7 +468,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -531,7 +525,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
@@ -558,7 +551,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1)
.addValues(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1024L, 8L, 1024L, 8L, 0L)
.addValues(TEST_IFACE, UID_RED, SET_DEFAULT, 0xF00D, 512L, 4L, 512L, 4L, 0L));
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
forcePollAndWaitForIdle();
@@ -588,7 +580,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -646,7 +637,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -690,7 +680,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -740,7 +729,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -797,7 +785,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState(true /* isMetered */)};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -837,7 +824,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
new NetworkState[] {buildMobile3gState(IMSI_1, true /* isRoaming */)};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
@@ -875,7 +861,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
@@ -916,7 +901,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
NetworkState[] states = new NetworkState[] {buildWifiState()};
expectNetworkStatsSummary(buildEmptyStats());
expectNetworkStatsUidDetail(buildEmptyStats());
expectBandwidthControlCheck();
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
@@ -1057,7 +1041,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
private void expectSystemReady() throws Exception {
expectNetworkStatsSummary(buildEmptyStats());
expectBandwidthControlCheck();
}
private String getActiveIface(NetworkState... states) throws Exception {
@@ -1125,10 +1108,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
when(mSettings.getUidTagPersistBytes(anyLong())).thenReturn(MB_IN_BYTES);
}
private void expectBandwidthControlCheck() throws Exception {
when(mNetManager.isBandwidthControlEnabled()).thenReturn(true);
}
private void assertStatsFilesExist(boolean exist) {
final File basePath = new File(mStatsDir, "netstats");
if (exist) {