LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Disable KeepaliveStatsTracker instead of throwing.

Browse Source 
Avoid throwing errors when something unexpected occurs with the
keepalive state since metrics should not cause a crash. Instead, disable
the tracker and skip writing the metrics.

Bug: 288059409
Test: atest FrameworksNetTests
Change-Id: I1cd5acb32eb062ccdf7d1ac3e25a21309ad011c2
This commit is contained in:
Hansen Kurli
2023-06-20 09:50:06 +00:00
parent a58320fefc
commit 21eae05622
3 changed files with 96 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
View File

@@ -338,6 +338,10 @@ public class AutomaticOnOffKeepaliveTracker {
}
private void writeMetricsAndRescheduleAlarm() {
// If the metrics is disabled, skip writing and scheduling the next alarm.
if (!mKeepaliveStatsTracker.isEnabled()) {
return;
}
mKeepaliveStatsTracker.writeAndResetMetrics();
final long time = mDependencies.getElapsedRealtime();

86
service/src/com/android/server/connectivity/KeepaliveStatsTracker.java
View File

@@ -56,6 +56,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
/**
* Tracks carrier and duration metrics of automatic on/off keepalives.
@@ -63,9 +64,14 @@ import java.util.Set;
* <p>This class follows AutomaticOnOffKeepaliveTracker closely and its on*Keepalive methods needs
* to be called in a timely manner to keep the metrics accurate. It is also not thread-safe and all
* public methods must be called by the same thread, namely the ConnectivityService handler thread.
*
* <p>In the case that the keepalive state becomes out of sync with the hardware, the tracker will
* be disabled. e.g. Calling onStartKeepalive on a given network, slot pair twice without calling
* onStopKeepalive is unexpected and will disable the tracker.
*/
public class KeepaliveStatsTracker {
private static final String TAG = KeepaliveStatsTracker.class.getSimpleName();
private static final int INVALID_KEEPALIVE_ID = -1;
@NonNull private final Handler mConnectivityServiceHandler;
@NonNull private final Dependencies mDependencies;
@@ -76,6 +82,10 @@ public class KeepaliveStatsTracker {
// Updates are received from the ACTION_DEFAULT_SUBSCRIPTION_CHANGED broadcast.
private int mCachedDefaultSubscriptionId = SubscriptionManager.INVALID_SUBSCRIPTION_ID;
// Boolean to track whether the KeepaliveStatsTracker is enabled.
// Use a final AtomicBoolean to ensure initialization is seen on the handler thread.
private final AtomicBoolean mEnabled = new AtomicBoolean(true);
// Class to store network information, lifetime durations and active state of a keepalive.
private static final class KeepaliveStats {
// The carrier ID for a keepalive, or TelephonyManager.UNKNOWN_CARRIER_ID(-1) if not set.
@@ -185,17 +195,21 @@ public class KeepaliveStatsTracker {
// Map of keepalives identified by the id from getKeepaliveId to their stats information.
private final SparseArray<KeepaliveStats> mKeepaliveStatsPerId = new SparseArray<>();
// Generate a unique integer using a given network's netId and the slot number.
// Generate and return a unique integer using a given network's netId and the slot number.
// This is possible because netId is a 16 bit integer, so an integer with the first 16 bits as
// the netId and the last 16 bits as the slot number can be created. This allows slot numbers to
// be up to 2^16.
// Returns INVALID_KEEPALIVE_ID if the netId or slot is not as expected above.
private int getKeepaliveId(@NonNull Network network, int slot) {
final int netId = network.getNetId();
// Since there is no enforcement that a Network's netId is valid check for it here.
if (netId < 0 || netId >= (1 << 16)) {
throw new IllegalArgumentException("Unexpected netId value: " + netId);
disableTracker("Unexpected netId value: " + netId);
return INVALID_KEEPALIVE_ID;
}
if (slot < 0 || slot >= (1 << 16)) {
throw new IllegalArgumentException("Unexpected slot value: " + slot);
disableTracker("Unexpected slot value: " + slot);
return INVALID_KEEPALIVE_ID;
}
return (netId << 16) + slot;
@@ -332,12 +346,12 @@ public class KeepaliveStatsTracker {
/** Ensures the list of duration metrics is large enough for number of registered keepalives. */
private void ensureDurationPerNumOfKeepaliveSize() {
if (mNumActiveKeepalive < 0 || mNumRegisteredKeepalive < 0) {
throw new IllegalStateException(
"Number of active or registered keepalives is negative");
disableTracker("Number of active or registered keepalives is negative");
return;
}
if (mNumActiveKeepalive > mNumRegisteredKeepalive) {
throw new IllegalStateException(
"Number of active keepalives greater than registered keepalives");
disableTracker("Number of active keepalives greater than registered keepalives");
return;
}
while (mDurationPerNumOfKeepalive.size() <= mNumRegisteredKeepalive) {
@@ -426,10 +440,12 @@ public class KeepaliveStatsTracker {
int appUid,
boolean isAutoKeepalive) {
ensureRunningOnHandlerThread();
if (!isEnabled()) return;
final int keepaliveId = getKeepaliveId(network, slot);
if (keepaliveId == INVALID_KEEPALIVE_ID) return;
if (mKeepaliveStatsPerId.contains(keepaliveId)) {
throw new IllegalArgumentException(
"Attempt to start keepalive stats on a known network, slot pair");
disableTracker("Attempt to start keepalive stats on a known network, slot pair");
return;
}
mNumKeepaliveRequests++;
@@ -457,13 +473,11 @@ public class KeepaliveStatsTracker {
/**
* Inform the KeepaliveStatsTracker that the keepalive with the given network, slot pair has
* updated its active state to keepaliveActive.
*
* @return the KeepaliveStats associated with the network, slot pair or null if it is unknown.
*/
private @NonNull KeepaliveStats onKeepaliveActive(
private void onKeepaliveActive(
@NonNull Network network, int slot, boolean keepaliveActive) {
final long timeNow = mDependencies.getElapsedRealtime();
return onKeepaliveActive(network, slot, keepaliveActive, timeNow);
onKeepaliveActive(network, slot, keepaliveActive, timeNow);
}
/**
@@ -474,45 +488,53 @@ public class KeepaliveStatsTracker {
* @param slot the slot number of the keepalive
* @param keepaliveActive the new active state of the keepalive
* @param timeNow a timestamp obtained using Dependencies.getElapsedRealtime
* @return the KeepaliveStats associated with the network, slot pair or null if it is unknown.
*/
private @NonNull KeepaliveStats onKeepaliveActive(
private void onKeepaliveActive(
@NonNull Network network, int slot, boolean keepaliveActive, long timeNow) {
ensureRunningOnHandlerThread();
final int keepaliveId = getKeepaliveId(network, slot);
if (!mKeepaliveStatsPerId.contains(keepaliveId)) {
throw new IllegalArgumentException(
"Attempt to set active keepalive on an unknown network, slot pair");
if (keepaliveId == INVALID_KEEPALIVE_ID) return;
final KeepaliveStats keepaliveStats = mKeepaliveStatsPerId.get(keepaliveId, null);
if (keepaliveStats == null) {
disableTracker("Attempt to set active keepalive on an unknown network, slot pair");
return;
}
updateDurationsPerNumOfKeepalive(timeNow);
final KeepaliveStats keepaliveStats = mKeepaliveStatsPerId.get(keepaliveId);
if (keepaliveActive != keepaliveStats.isKeepaliveActive()) {
mNumActiveKeepalive += keepaliveActive ? 1 : -1;
}
keepaliveStats.updateLifetimeStatsAndSetActive(timeNow, keepaliveActive);
return keepaliveStats;
}
/** Inform the KeepaliveStatsTracker a keepalive has just been paused. */
public void onPauseKeepalive(@NonNull Network network, int slot) {
ensureRunningOnHandlerThread();
if (!isEnabled()) return;
onKeepaliveActive(network, slot, /* keepaliveActive= */ false);
}
/** Inform the KeepaliveStatsTracker a keepalive has just been resumed. */
public void onResumeKeepalive(@NonNull Network network, int slot) {
ensureRunningOnHandlerThread();
if (!isEnabled()) return;
onKeepaliveActive(network, slot, /* keepaliveActive= */ true);
}
/** Inform the KeepaliveStatsTracker a keepalive has just been stopped. */
public void onStopKeepalive(@NonNull Network network, int slot) {
ensureRunningOnHandlerThread();
if (!isEnabled()) return;
final int keepaliveId = getKeepaliveId(network, slot);
if (keepaliveId == INVALID_KEEPALIVE_ID) return;
final long timeNow = mDependencies.getElapsedRealtime();
final KeepaliveStats keepaliveStats =
onKeepaliveActive(network, slot, /* keepaliveActive= */ false, timeNow);
final KeepaliveStats keepaliveStats = mKeepaliveStatsPerId.get(keepaliveId, null);
if (keepaliveStats == null) return;
mNumRegisteredKeepalive--;
@@ -651,6 +673,20 @@ public class KeepaliveStatsTracker {
return metrics;
}
private void disableTracker(String msg) {
if (!mEnabled.compareAndSet(/* expectedValue= */ true, /* newValue= */ false)) {
// already disabled
return;
}
Log.wtf(TAG, msg + ". Disabling KeepaliveStatsTracker");
// TODO: Unregister the OnSubscriptionsChangedListener and BroadcastReceiver.
}
/** Whether this tracker is enabled. This method is thread safe. */
public boolean isEnabled() {
return mEnabled.get();
}
/** Writes the stored metrics to ConnectivityStatsLog and resets. */
public void writeAndResetMetrics() {
ensureRunningOnHandlerThread();
@@ -660,6 +696,10 @@ public class KeepaliveStatsTracker {
Log.d(TAG, "KeepaliveStatsTracker is disabled before T, skipping write");
return;
}
if (!isEnabled()) {
Log.d(TAG, "KeepaliveStatsTracker is disabled, skipping write");
return;
}
final DailykeepaliveInfoReported dailyKeepaliveInfoReported = buildAndResetMetrics();
mDependencies.writeStats(dailyKeepaliveInfoReported);

28
tests/unit/java/com/android/server/connectivity/KeepaliveStatsTrackerTest.java
View File

@@ -25,7 +25,9 @@ import static com.android.testutils.HandlerUtils.visibleOnHandlerThread;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
@@ -433,6 +435,12 @@ public class KeepaliveStatsTrackerTest {
assertCarrierLifetimeMetrics(expectKeepaliveCarrierStatsArray, actualCarrierLifetime);
}
// The KeepaliveStatsTracker will be disabled when an error occurs with the keepalive states.
// Most tests should assert that the tracker is still active to ensure no errors occurred.
private void assertKeepaliveStatsTrackerActive() {
assertTrue(mKeepaliveStatsTracker.isEnabled());
}
@Test
public void testNoKeepalive() {
final int writeTime = 5000;
@@ -452,6 +460,7 @@ public class KeepaliveStatsTrackerTest {
expectRegisteredDurations,
expectActiveDurations,
new KeepaliveCarrierStats[0]);
assertKeepaliveStatsTrackerActive();
}
/*
@@ -485,6 +494,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -523,6 +533,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -567,6 +578,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -615,6 +627,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -657,6 +670,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -708,6 +722,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -788,6 +803,7 @@ public class KeepaliveStatsTrackerTest {
expectRegisteredDurations[1] + 2 * expectRegisteredDurations[2],
expectActiveDurations[1] + 2 * expectActiveDurations[2])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -857,6 +873,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations2[1], expectActiveDurations2[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -946,6 +963,7 @@ public class KeepaliveStatsTrackerTest {
expectRegisteredDurations2,
expectActiveDurations2,
new KeepaliveCarrierStats[] {expectKeepaliveCarrierStats3});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -957,7 +975,10 @@ public class KeepaliveStatsTrackerTest {
onStartKeepalive(startTime1, TEST_SLOT);
// Attempt to use the same (network, slot)
assertThrows(IllegalArgumentException.class, () -> onStartKeepalive(startTime2, TEST_SLOT));
onStartKeepalive(startTime2, TEST_SLOT);
// Starting a 2nd keepalive on the same slot is unexpected and an error so the stats tracker
// is disabled.
assertFalse(mKeepaliveStatsTracker.isEnabled());
final DailykeepaliveInfoReported dailyKeepaliveInfoReported =
buildKeepaliveMetrics(writeTime);
@@ -1018,6 +1039,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1065,6 +1087,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
expectKeepaliveCarrierStats1, expectKeepaliveCarrierStats2
});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1106,6 +1129,7 @@ public class KeepaliveStatsTrackerTest {
/* expectRegisteredDurations= */ new int[] {startTime, writeTime - startTime},
/* expectActiveDurations= */ new int[] {startTime, writeTime - startTime},
new KeepaliveCarrierStats[] {expectKeepaliveCarrierStats});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1148,6 +1172,7 @@ public class KeepaliveStatsTrackerTest {
writeTime * 3 - startTime1 - startTime2 - startTime3,
writeTime * 3 - startTime1 - startTime2 - startTime3)
});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1197,6 +1222,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
expectKeepaliveCarrierStats1, expectKeepaliveCarrierStats2
});
assertKeepaliveStatsTrackerActive();
}
@Test