Unregister DeviceIdle Receiver in IpSecPacketLossDetector
Ensure that when the IpSecPacketLossDetector cleans up due to the
underlying network changing, that the broadcast receiver it's listening
to is cleaned up. Previously this would leak broadcast receivers.
Based on inspection, all instances that touch the BroadcastReceiver
are done on the same looper, so we shouldn't need to guard the creation
and destruction of the receiver.
Also, stop closing the monitored IpSecTransform. The monitor should not
have been closing the transform in the first place. Most of the time it
isn't an issue; however, in the event of MOBIKE, the transform will
remain open, so this is a functional bug.
Bug: 389208101
Test: atest IpSecPacketLossDetectorTest:testClose
Flag: EXEMPT bugfix
Change-Id: I26245960c36eaa0557d377b1672cc76ab0f70b03
diff --git a/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/IpSecPacketLossDetector.java b/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/IpSecPacketLossDetector.java
index 6467af4..c8c645f 100644
--- a/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/IpSecPacketLossDetector.java
+++ b/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/IpSecPacketLossDetector.java
@@ -138,6 +138,8 @@
@NonNull private final Object mCancellationToken = new Object();
@NonNull private final PacketLossCalculator mPacketLossCalculator;
+ @Nullable private BroadcastReceiver mDeviceIdleReceiver;
+
@Nullable private IpSecTransformWrapper mInboundTransform;
@Nullable private IpSecTransformState mLastIpSecTransformState;
@@ -168,19 +170,21 @@
// Register for system broadcasts to monitor idle mode change
final IntentFilter intentFilter = new IntentFilter();
intentFilter.addAction(PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED);
+
+ mDeviceIdleReceiver = new BroadcastReceiver() {
+ @Override
+ public void onReceive(Context context, Intent intent) {
+ if (PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED.equals(
+ intent.getAction())
+ && mPowerManager.isDeviceIdleMode()) {
+ mLastIpSecTransformState = null;
+ }
+ }
+ };
getVcnContext()
.getContext()
.registerReceiver(
- new BroadcastReceiver() {
- @Override
- public void onReceive(Context context, Intent intent) {
- if (PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED.equals(
- intent.getAction())
- && mPowerManager.isDeviceIdleMode()) {
- mLastIpSecTransformState = null;
- }
- }
- },
+ mDeviceIdleReceiver,
intentFilter,
null /* broadcastPermission not required */,
mHandler);
@@ -338,7 +342,12 @@
super.close();
if (mInboundTransform != null) {
- mInboundTransform.close();
+ mInboundTransform = null;
+ }
+
+ if (mDeviceIdleReceiver != null) {
+ getVcnContext().getContext().unregisterReceiver(mDeviceIdleReceiver);
+ mDeviceIdleReceiver = null;
}
}
diff --git a/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/NetworkMetricMonitor.java b/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/NetworkMetricMonitor.java
index 1485344..55829a5 100644
--- a/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/NetworkMetricMonitor.java
+++ b/packages/Vcn/service-b/src/com/android/server/vcn/routeselection/NetworkMetricMonitor.java
@@ -165,7 +165,7 @@
}
}
- /** Set the IpSecTransform that applied to the Network being monitored */
+ /** Set the IpSecTransform that is applied to the Network being monitored */
public void setInboundTransform(@NonNull IpSecTransform inTransform) {
setInboundTransformInternal(new IpSecTransformWrapper(inTransform));
}
diff --git a/tests/vcn/java/com/android/server/vcn/routeselection/IpSecPacketLossDetectorTest.java b/tests/vcn/java/com/android/server/vcn/routeselection/IpSecPacketLossDetectorTest.java
index 5db02e3..7b1e4cc 100644
--- a/tests/vcn/java/com/android/server/vcn/routeselection/IpSecPacketLossDetectorTest.java
+++ b/tests/vcn/java/com/android/server/vcn/routeselection/IpSecPacketLossDetectorTest.java
@@ -38,6 +38,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -284,8 +285,11 @@
// Stop the monitor
mIpSecPacketLossDetector.close();
+ mIpSecPacketLossDetector.close();
verifyStopped();
- verify(mIpSecTransform).close();
+
+ verify(mIpSecTransform, never()).close();
+ verify(mContext).unregisterReceiver(any());
}
@Test
