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

Commit a4f28eda authored by Benedict Wong's avatar Benedict Wong
Browse files

Ensure teardown() only triggered on the right NetworkAgent. 

This change ensures that teardown() is only ever triggered when the
NetworkAgent matches the one that the unwanted() callback is triggered
on. This ensures that in cases where the unwanted() is called as a
result of unregister(), or even after a new NetworkAgent is created (and
stored in the mNetworkAgent field), the teardown does not occur.

Bug: 184304972
Test: atest FrameworksVcnTests
Change-Id: Ic8ab66ff1e48d43be784974cda7574d0191e557f
parent 16952dce

services/core/java/com/android/server/vcn/VcnGatewayConnection.java

+16 −10
Original line number Diff line number Diff line
@@ -1473,15 +1473,21 @@ public class VcnGatewayConnection extends StateMachine { 
                            Vcn.getNetworkScore(),

                            nac,

                            mVcnContext.getVcnNetworkProvider(),

                            () -> {

                            (agentRef) -> {

                                // Only trigger teardown if the NetworkAgent hasn't been replaced or

                                // changed. This guards against two cases - the first where

                                // unwanted() may be called as a result of the

                                // NetworkAgent.unregister() call, which might trigger a teardown

                                // instead of just a Network disconnect, as well as the case where a

                                // new NetworkAgent replaces an old one before the unwanted() call

                                // is processed.

                                if (mNetworkAgent != agentRef) {

                                    Slog.d(TAG, "unwanted() called on stale NetworkAgent");

                                    return;

                                }



                                Slog.d(TAG, "NetworkAgent was unwanted");

                                // If network agent has already been torn down, skip sending the

                                // disconnect. Unwanted() is always called, even when networkAgents

                                // are unregistered in teardownNetwork(), so prevent duplicate

                                // notifications.

                                if (mNetworkAgent != null) {

                                teardownAsynchronously();

                                }

                            } /* networkUnwantedCallback */,

                            (status) -> {

                                if (status == NetworkAgent.VALIDATION_STATUS_VALID) {
@@ -2089,7 +2095,7 @@ public class VcnGatewayConnection extends StateMachine { 
                @NonNull int score,

                @NonNull NetworkAgentConfig nac,

                @NonNull NetworkProvider provider,

                @NonNull Runnable networkUnwantedCallback,

                @NonNull Consumer<NetworkAgent> networkUnwantedCallback,

                @NonNull Consumer<Integer> validationStatusCallback) {

            return new NetworkAgent(

                    vcnContext.getContext(),
@@ -2102,7 +2108,7 @@ public class VcnGatewayConnection extends StateMachine { 
                    provider) {

                @Override

                public void onNetworkUnwanted() {

                    networkUnwantedCallback.run();

                    networkUnwantedCallback.accept(this);

                }



                @Override

tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java

+60 −0
Original line number Diff line number Diff line
@@ -323,6 +323,66 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection 
        assertFalse(mGatewayConnection.isInSafeMode());

    }



    private Consumer<NetworkAgent> setupNetworkAndGetUnwantedCallback() {

        triggerChildOpened();

        mTestLooper.dispatchAll();



        final ArgumentCaptor<Consumer<NetworkAgent>> unwantedCallbackCaptor =

                ArgumentCaptor.forClass(Consumer.class);

        verify(mDeps)

                .newNetworkAgent(

                        any(),

                        any(),

                        any(),

                        any(),

                        anyInt(),

                        any(),

                        any(),

                        unwantedCallbackCaptor.capture(),

                        any());



        return unwantedCallbackCaptor.getValue();

    }



    @Test

    public void testUnwantedNetworkAgentTriggersTeardown() throws Exception {

        final Consumer<NetworkAgent> unwantedCallback = setupNetworkAndGetUnwantedCallback();



        unwantedCallback.accept(mNetworkAgent);

        mTestLooper.dispatchAll();



        assertTrue(mGatewayConnection.isQuitting());

        assertEquals(mGatewayConnection.mDisconnectingState, mGatewayConnection.getCurrentState());

    }



    @Test

    public void testUnwantedNetworkAgentWithDisconnectedNetworkAgent() throws Exception {

        final Consumer<NetworkAgent> unwantedCallback = setupNetworkAndGetUnwantedCallback();



        mGatewayConnection.setNetworkAgent(null);

        unwantedCallback.accept(mNetworkAgent);

        mTestLooper.dispatchAll();



        // Verify that the call was ignored; the state machine is still running, and the state has

        // not changed.

        assertFalse(mGatewayConnection.isQuitting());

        assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState());

    }



    @Test

    public void testUnwantedNetworkAgentWithNewNetworkAgent() throws Exception {

        final Consumer<NetworkAgent> unwantedCallback = setupNetworkAndGetUnwantedCallback();

        final NetworkAgent testAgent = mock(NetworkAgent.class);



        mGatewayConnection.setNetworkAgent(testAgent);

        unwantedCallback.accept(mNetworkAgent);

        mTestLooper.dispatchAll();



        assertFalse(mGatewayConnection.isQuitting());

        assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState());

        assertEquals(testAgent, mGatewayConnection.getNetworkAgent());

    }



    @Test

    public void testChildSessionClosedTriggersDisconnect() throws Exception {

        // Verify scheduled but not canceled when entering ConnectedState