NetworkStats: Fix race condition causing system server crashes
NetworkStatsService uses an internal boolean to know when it has started for the purpose of preventing access to other internal variables before they are initialized. However that boolean is set to true in systemReady() non-atomically with respect to the initialization of the other variables it guards, which can cause the system server to crash. This patch fixes this concurrency bug by moving setting the internal boolean flag and the variable it guards in one atomic synchronized block. This patch also removes code checking if bandwidth control is enabled, because this is now always true. Bug: 132767673 Test: Compiled. Change-Id: Ia089b5767ce271d669879c975508654d4dd03429
This commit is contained in:
@@ -239,7 +239,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -283,7 +282,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -357,7 +355,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -398,7 +395,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
|
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
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, TAG_NONE, 1536L, 12L, 512L, 4L, 0L)
|
||||||
.addValues(TEST_IFACE, UID_RED, SET_DEFAULT, 0xF00D, 512L, 4L, 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));
|
.addValues(TEST_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 512L, 4L, 0L, 0L, 0L));
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
||||||
forcePollAndWaitForIdle();
|
forcePollAndWaitForIdle();
|
||||||
@@ -473,7 +468,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -531,7 +525,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
|
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -558,7 +551,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1)
|
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, TAG_NONE, 1024L, 8L, 1024L, 8L, 0L)
|
||||||
.addValues(TEST_IFACE, UID_RED, SET_DEFAULT, 0xF00D, 512L, 4L, 512L, 4L, 0L));
|
.addValues(TEST_IFACE, UID_RED, SET_DEFAULT, 0xF00D, 512L, 4L, 512L, 4L, 0L));
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
||||||
forcePollAndWaitForIdle();
|
forcePollAndWaitForIdle();
|
||||||
@@ -588,7 +580,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -646,7 +637,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -690,7 +680,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
|
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -740,7 +729,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -797,7 +785,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState(true /* isMetered */)};
|
NetworkState[] states = new NetworkState[] {buildWifiState(true /* isMetered */)};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -837,7 +824,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
new NetworkState[] {buildMobile3gState(IMSI_1, true /* isRoaming */)};
|
new NetworkState[] {buildMobile3gState(IMSI_1, true /* isRoaming */)};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -875,7 +861,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
|
NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -916,7 +901,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
NetworkState[] states = new NetworkState[] {buildWifiState()};
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectNetworkStatsUidDetail(buildEmptyStats());
|
expectNetworkStatsUidDetail(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
|
|
||||||
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
mService.forceUpdateIfaces(NETWORKS_WIFI, states, getActiveIface(states));
|
||||||
|
|
||||||
@@ -1057,7 +1041,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
|
|
||||||
private void expectSystemReady() throws Exception {
|
private void expectSystemReady() throws Exception {
|
||||||
expectNetworkStatsSummary(buildEmptyStats());
|
expectNetworkStatsSummary(buildEmptyStats());
|
||||||
expectBandwidthControlCheck();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private String getActiveIface(NetworkState... states) throws Exception {
|
private String getActiveIface(NetworkState... states) throws Exception {
|
||||||
@@ -1125,10 +1108,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
|
|||||||
when(mSettings.getUidTagPersistBytes(anyLong())).thenReturn(MB_IN_BYTES);
|
when(mSettings.getUidTagPersistBytes(anyLong())).thenReturn(MB_IN_BYTES);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void expectBandwidthControlCheck() throws Exception {
|
|
||||||
when(mNetManager.isBandwidthControlEnabled()).thenReturn(true);
|
|
||||||
}
|
|
||||||
|
|
||||||
private void assertStatsFilesExist(boolean exist) {
|
private void assertStatsFilesExist(boolean exist) {
|
||||||
final File basePath = new File(mStatsDir, "netstats");
|
final File basePath = new File(mStatsDir, "netstats");
|
||||||
if (exist) {
|
if (exist) {
|
||||||
|
|||||||
Reference in New Issue
Block a user