Merge changes Ia5fc3e53,Ie1ab6013,I1cd5acb3 into main

* changes:
  Unregister BroadcastReceiver and OnSubscriptionsChangedListener.
  Disable KeepaliveStatsTracker before T.
  Disable KeepaliveStatsTracker instead of throwing.
This commit is contained in:
Hansen Kurli
2023-07-28 11:20:21 +00:00
committed by Gerrit Code Review
3 changed files with 181 additions and 65 deletions

View File

@@ -332,12 +332,18 @@ public class AutomaticOnOffKeepaliveTracker {
final long time = mDependencies.getElapsedRealtime();
mMetricsWriteTimeBase = time % METRICS_COLLECTION_DURATION_MS;
final long triggerAtMillis = mMetricsWriteTimeBase + METRICS_COLLECTION_DURATION_MS;
mAlarmManager.set(AlarmManager.ELAPSED_REALTIME_WAKEUP, triggerAtMillis, TAG,
this::writeMetricsAndRescheduleAlarm, handler);
if (mKeepaliveStatsTracker.isEnabled()) {
final long triggerAtMillis = mMetricsWriteTimeBase + METRICS_COLLECTION_DURATION_MS;
mAlarmManager.set(AlarmManager.ELAPSED_REALTIME_WAKEUP, triggerAtMillis, TAG,
this::writeMetricsAndRescheduleAlarm, handler);
}
}
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();

View File

@@ -56,6 +56,8 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicBoolean;
/**
* Tracks carrier and duration metrics of automatic on/off keepalives.
@@ -63,9 +65,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 +83,11 @@ 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.
// Repeated fields in metrics are only supported on T+ so this is enabled only on T+.
private final AtomicBoolean mEnabled = new AtomicBoolean(SdkLevel.isAtLeastT());
// 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 +197,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;
@@ -274,29 +290,42 @@ public class KeepaliveStatsTracker {
this(context, handler, new Dependencies());
}
private final Context mContext;
private final SubscriptionManager mSubscriptionManager;
private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
mCachedDefaultSubscriptionId =
intent.getIntExtra(
SubscriptionManager.EXTRA_SUBSCRIPTION_INDEX,
SubscriptionManager.INVALID_SUBSCRIPTION_ID);
}
};
private final CompletableFuture<OnSubscriptionsChangedListener> mListenerFuture =
new CompletableFuture<>();
@VisibleForTesting
public KeepaliveStatsTracker(
@NonNull Context context,
@NonNull Handler handler,
@NonNull Dependencies dependencies) {
Objects.requireNonNull(context);
mContext = Objects.requireNonNull(context);
mDependencies = Objects.requireNonNull(dependencies);
mConnectivityServiceHandler = Objects.requireNonNull(handler);
final SubscriptionManager subscriptionManager =
mSubscriptionManager =
Objects.requireNonNull(context.getSystemService(SubscriptionManager.class));
mLastUpdateDurationsTimestamp = mDependencies.getElapsedRealtime();
if (!isEnabled()) {
return;
}
context.registerReceiver(
new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
mCachedDefaultSubscriptionId =
intent.getIntExtra(
SubscriptionManager.EXTRA_SUBSCRIPTION_INDEX,
SubscriptionManager.INVALID_SUBSCRIPTION_ID);
}
},
mBroadcastReceiver,
new IntentFilter(SubscriptionManager.ACTION_DEFAULT_SUBSCRIPTION_CHANGED),
/* broadcastPermission= */ null,
mConnectivityServiceHandler);
@@ -306,38 +335,41 @@ public class KeepaliveStatsTracker {
// this will throw. Therefore, post a runnable that creates it there.
// When the callback is called on the BackgroundThread, post a message on the CS handler
// thread to update the caches, which can only be touched there.
BackgroundThread.getHandler().post(() ->
subscriptionManager.addOnSubscriptionsChangedListener(
r -> r.run(), new OnSubscriptionsChangedListener() {
@Override
public void onSubscriptionsChanged() {
final List<SubscriptionInfo> activeSubInfoList =
subscriptionManager.getActiveSubscriptionInfoList();
// A null subInfo list here indicates the current state is unknown
// but not necessarily empty, simply ignore it. Another call to the
// listener will be invoked in the future.
if (activeSubInfoList == null) return;
mConnectivityServiceHandler.post(() -> {
mCachedCarrierIdPerSubId.clear();
BackgroundThread.getHandler().post(() -> {
final OnSubscriptionsChangedListener listener =
new OnSubscriptionsChangedListener() {
@Override
public void onSubscriptionsChanged() {
final List<SubscriptionInfo> activeSubInfoList =
mSubscriptionManager.getActiveSubscriptionInfoList();
// A null subInfo list here indicates the current state is unknown
// but not necessarily empty, simply ignore it. Another call to the
// listener will be invoked in the future.
if (activeSubInfoList == null) return;
mConnectivityServiceHandler.post(() -> {
mCachedCarrierIdPerSubId.clear();
for (final SubscriptionInfo subInfo : activeSubInfoList) {
mCachedCarrierIdPerSubId.put(subInfo.getSubscriptionId(),
subInfo.getCarrierId());
}
});
}
}));
for (final SubscriptionInfo subInfo : activeSubInfoList) {
mCachedCarrierIdPerSubId.put(subInfo.getSubscriptionId(),
subInfo.getCarrierId());
}
});
}
};
mListenerFuture.complete(listener);
mSubscriptionManager.addOnSubscriptionsChangedListener(r -> r.run(), listener);
});
}
/** 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 +458,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 +491,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 +506,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);
onKeepaliveActive(network, slot, /* keepaliveActive= */ false, timeNow);
final KeepaliveStats keepaliveStats = mKeepaliveStatsPerId.get(keepaliveId, null);
if (keepaliveStats == null) return;
mNumRegisteredKeepalive--;
@@ -651,7 +691,25 @@ public class KeepaliveStatsTracker {
return metrics;
}
/** Writes the stored metrics to ConnectivityStatsLog and resets. */
private void disableTracker(String msg) {
if (!mEnabled.compareAndSet(/* expectedValue= */ true, /* newValue= */ false)) {
// already disabled
return;
}
Log.wtf(TAG, msg + ". Disabling KeepaliveStatsTracker");
mContext.unregisterReceiver(mBroadcastReceiver);
// The returned future is ignored since it is void and the is never completed exceptionally.
final CompletableFuture<Void> unused = mListenerFuture.thenAcceptAsync(
listener -> mSubscriptionManager.removeOnSubscriptionsChangedListener(listener),
BackgroundThread.getExecutor());
}
/** 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();
// Keepalive stats use repeated atoms, which are only supported on T+. If written to statsd
@@ -660,6 +718,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);

View File

@@ -25,10 +25,13 @@ 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;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.doReturn;
@@ -118,16 +121,25 @@ public class KeepaliveStatsTrackerTest {
@Mock private KeepaliveStatsTracker.Dependencies mDependencies;
@Mock private SubscriptionManager mSubscriptionManager;
private void triggerBroadcastDefaultSubId(int subId) {
private BroadcastReceiver getBroadcastReceiver() {
final ArgumentCaptor<BroadcastReceiver> receiverCaptor =
ArgumentCaptor.forClass(BroadcastReceiver.class);
verify(mContext).registerReceiver(receiverCaptor.capture(), /* filter= */ any(),
/* broadcastPermission= */ any(), eq(mTestHandler));
verify(mContext).registerReceiver(
receiverCaptor.capture(),
argThat(intentFilter -> intentFilter.matchAction(
SubscriptionManager.ACTION_DEFAULT_SUBSCRIPTION_CHANGED)),
/* broadcastPermission= */ any(),
eq(mTestHandler));
return receiverCaptor.getValue();
}
private void triggerBroadcastDefaultSubId(int subId) {
final Intent intent =
new Intent(TelephonyManager.ACTION_SUBSCRIPTION_CARRIER_IDENTITY_CHANGED);
new Intent(SubscriptionManager.ACTION_DEFAULT_SUBSCRIPTION_CHANGED);
intent.putExtra(SubscriptionManager.EXTRA_SUBSCRIPTION_INDEX, subId);
receiverCaptor.getValue().onReceive(mContext, intent);
getBroadcastReceiver().onReceive(mContext, intent);
}
private OnSubscriptionsChangedListener getOnSubscriptionsChangedListener() {
@@ -433,6 +445,24 @@ 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());
}
private void assertKeepaliveStatsTrackerDisabled() {
assertFalse(mKeepaliveStatsTracker.isEnabled());
final OnSubscriptionsChangedListener listener = getOnSubscriptionsChangedListener();
// BackgroundThread will remove the OnSubscriptionsChangedListener.
HandlerUtils.waitForIdle(BackgroundThread.getHandler(), TIMEOUT_MS);
verify(mSubscriptionManager).removeOnSubscriptionsChangedListener(listener);
final BroadcastReceiver receiver = getBroadcastReceiver();
verify(mContext).unregisterReceiver(receiver);
}
@Test
public void testNoKeepalive() {
final int writeTime = 5000;
@@ -452,6 +482,7 @@ public class KeepaliveStatsTrackerTest {
expectRegisteredDurations,
expectActiveDurations,
new KeepaliveCarrierStats[0]);
assertKeepaliveStatsTrackerActive();
}
/*
@@ -485,6 +516,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -523,6 +555,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -567,6 +600,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -615,6 +649,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -657,6 +692,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -708,6 +744,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -788,6 +825,7 @@ public class KeepaliveStatsTrackerTest {
expectRegisteredDurations[1] + 2 * expectRegisteredDurations[2],
expectActiveDurations[1] + 2 * expectActiveDurations[2])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -857,6 +895,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations2[1], expectActiveDurations2[1])
});
assertKeepaliveStatsTrackerActive();
}
/*
@@ -946,6 +985,7 @@ public class KeepaliveStatsTrackerTest {
expectRegisteredDurations2,
expectActiveDurations2,
new KeepaliveCarrierStats[] {expectKeepaliveCarrierStats3});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -957,7 +997,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.
assertKeepaliveStatsTrackerDisabled();
final DailykeepaliveInfoReported dailyKeepaliveInfoReported =
buildKeepaliveMetrics(writeTime);
@@ -1018,6 +1061,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
getDefaultCarrierStats(expectRegisteredDurations[1], expectActiveDurations[1])
});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1065,6 +1109,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
expectKeepaliveCarrierStats1, expectKeepaliveCarrierStats2
});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1106,6 +1151,7 @@ public class KeepaliveStatsTrackerTest {
/* expectRegisteredDurations= */ new int[] {startTime, writeTime - startTime},
/* expectActiveDurations= */ new int[] {startTime, writeTime - startTime},
new KeepaliveCarrierStats[] {expectKeepaliveCarrierStats});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1148,6 +1194,7 @@ public class KeepaliveStatsTrackerTest {
writeTime * 3 - startTime1 - startTime2 - startTime3,
writeTime * 3 - startTime1 - startTime2 - startTime3)
});
assertKeepaliveStatsTrackerActive();
}
@Test
@@ -1197,6 +1244,7 @@ public class KeepaliveStatsTrackerTest {
new KeepaliveCarrierStats[] {
expectKeepaliveCarrierStats1, expectKeepaliveCarrierStats2
});
assertKeepaliveStatsTrackerActive();
}
@Test