Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d960d4b1 authored by Nathan Harold's avatar Nathan Harold
Browse files

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
parent f920cc5d

packages/Vcn/service-b/src/com/android/server/vcn/routeselection/IpSecPacketLossDetector.java

+20 −11
Original line number Diff line number Diff line
@@ -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,10 +170,8 @@ 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);

        getVcnContext()

                .getContext()

                .registerReceiver(

                        new BroadcastReceiver() {



        mDeviceIdleReceiver = new BroadcastReceiver() {

            @Override

            public void onReceive(Context context, Intent intent) {

                if (PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED.equals(
@@ -180,7 +180,11 @@ public class IpSecPacketLossDetector extends NetworkMetricMonitor { 
                    mLastIpSecTransformState = null;

                }

            }

                        },

        };

        getVcnContext()

                .getContext()

                .registerReceiver(

                        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;

        }

    }

packages/Vcn/service-b/src/com/android/server/vcn/routeselection/NetworkMetricMonitor.java

+1 −1
Original line number Diff line number Diff line
@@ -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));

    }

tests/vcn/java/com/android/server/vcn/routeselection/IpSecPacketLossDetectorTest.java

+5 −1
Original line number Diff line number Diff line
@@ -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