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

3 files changed, 26 insertions, 13 deletions

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 6467af4355f6..c8c645f1276d 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 @@ public class IpSecPacketLossDetector extends NetworkMetricMonitor {
     @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 @@ public class IpSecPacketLossDetector extends NetworkMetricMonitor {
         // 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 @@ public class IpSecPacketLossDetector extends NetworkMetricMonitor {
         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 14853440a129..55829a5fe978 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 @@ public abstract class NetworkMetricMonitor implements AutoCloseable {
         }
     }
 
-    /** 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 5db02e376f3d..7b1e4cc62dab 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.doReturn;
 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 @@ public class IpSecPacketLossDetectorTest extends NetworkEvaluationTestBase {
 
         // Stop the monitor
         mIpSecPacketLossDetector.close();
+        mIpSecPacketLossDetector.close();
         verifyStopped();
-        verify(mIpSecTransform).close();
+
+        verify(mIpSecTransform, never()).close();
+        verify(mContext).unregisterReceiver(any());
     }
 
     @Test