Merge "Fix dangling NetworkAgent"

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

@@ -518,7 +518,11 @@ public class Vcn extends Handler {
     }
 
     private String getLogPrefix() {
-        return "[" + LogUtils.getHashedSubscriptionGroup(mSubscriptionGroup) + "] ";
+        return "["
+                + LogUtils.getHashedSubscriptionGroup(mSubscriptionGroup)
+                + "-"
+                + System.identityHashCode(this)
+                + "] ";
     }
 
     private void logVdbg(String msg) {

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

@@ -714,6 +714,18 @@ public class VcnGatewayConnection extends StateMachine {
     protected void onQuitting() {
         logDbg("Quitting VcnGatewayConnection");
 
+        if (mNetworkAgent != null) {
+            logWtf("NetworkAgent was non-null in onQuitting");
+            mNetworkAgent.unregister();
+            mNetworkAgent = null;
+        }
+
+        if (mIkeSession != null) {
+            logWtf("IkeSession was non-null in onQuitting");
+            mIkeSession.kill();
+            mIkeSession = null;
+        }
+
         // No need to call setInterfaceDown(); the IpSecInterface is being fully torn down.
         if (mTunnelIface != null) {
             mTunnelIface.close();
@@ -1863,6 +1875,7 @@ public class VcnGatewayConnection extends StateMachine {
 
             if (mUnderlying == null) {
                 logWtf("Underlying network was null in retry state");
+                teardownNetwork();
                 transitionTo(mDisconnectedState);
             } else {
                 // Safe to blindly set up, as it is cancelled and cleared on exiting this state
@@ -1879,6 +1892,7 @@ public class VcnGatewayConnection extends StateMachine {
 
                     // If new underlying is null, all networks were lost; go back to disconnected.
                     if (mUnderlying == null) {
+                        teardownNetwork();
                         transitionTo(mDisconnectedState);
                         return;
                     } else if (oldUnderlying != null
@@ -2134,6 +2148,8 @@ public class VcnGatewayConnection extends StateMachine {
                 + LogUtils.getHashedSubscriptionGroup(mSubscriptionGroup)
                 + "-"
                 + mConnectionConfig.getGatewayConnectionName()
+                + "-"
+                + System.identityHashCode(this)
                 + "] ";
     }
 

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

@@ -16,10 +16,16 @@
 
 package com.android.server.vcn;
 
+import static com.android.server.vcn.VcnGatewayConnection.VcnNetworkAgent;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
@@ -33,16 +39,20 @@ import org.junit.runner.RunWith;
 @SmallTest
 public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnectionTestBase {
     private long mFirstRetryInterval;
+    private VcnNetworkAgent mNetworkAgent;
 
     @Before
     public void setUp() throws Exception {
         super.setUp();
 
         mFirstRetryInterval = mConfig.getRetryIntervalsMillis()[0];
+        mNetworkAgent = mock(VcnNetworkAgent.class);
 
         mGatewayConnection.setUnderlyingNetwork(TEST_UNDERLYING_NETWORK_RECORD_1);
         mGatewayConnection.transitionTo(mGatewayConnection.mRetryTimeoutState);
         mTestLooper.dispatchAll();
+
+        mGatewayConnection.setNetworkAgent(mNetworkAgent);
     }
 
     @Test
@@ -54,6 +64,9 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect
 
         assertEquals(mGatewayConnection.mConnectingState, mGatewayConnection.getCurrentState());
         verifyRetryTimeoutAlarmAndGetCallback(mFirstRetryInterval, true /* expectCanceled */);
+
+        assertNotNull(mGatewayConnection.getNetworkAgent());
+        verify(mNetworkAgent, never()).unregister();
     }
 
     @Test
@@ -65,6 +78,9 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect
 
         assertEquals(mGatewayConnection.mRetryTimeoutState, mGatewayConnection.getCurrentState());
         verifyRetryTimeoutAlarmAndGetCallback(mFirstRetryInterval, false /* expectCanceled */);
+
+        assertNotNull(mGatewayConnection.getNetworkAgent());
+        verify(mNetworkAgent, never()).unregister();
     }
 
     @Test
@@ -76,6 +92,9 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect
 
         assertEquals(mGatewayConnection.mDisconnectedState, mGatewayConnection.getCurrentState());
         verifyRetryTimeoutAlarmAndGetCallback(mFirstRetryInterval, true /* expectCanceled */);
+
+        assertNull(mGatewayConnection.getNetworkAgent());
+        verify(mNetworkAgent).unregister();
     }
 
     @Test
@@ -93,6 +112,9 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect
 
         assertEquals(mGatewayConnection.mConnectingState, mGatewayConnection.getCurrentState());
         verifyRetryTimeoutAlarmAndGetCallback(mFirstRetryInterval, true /* expectCanceled */);
+
+        assertNotNull(mGatewayConnection.getNetworkAgent());
+        verify(mNetworkAgent, never()).unregister();
     }
 
     @Test
@@ -108,6 +130,9 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect
 
         assertNull(mGatewayConnection.getCurrentState());
         assertTrue(mGatewayConnection.isQuitting());
+
+        assertNull(mGatewayConnection.getNetworkAgent());
+        verify(mNetworkAgent).unregister();
     }
 
     @Test
@@ -117,5 +142,8 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect
 
         assertEquals(mGatewayConnection.mDisconnectedState, mGatewayConnection.getCurrentState());
         assertFalse(mGatewayConnection.isQuitting());
+
+        assertNull(mGatewayConnection.getNetworkAgent());
+        verify(mNetworkAgent).unregister();
     }
 }

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

@@ -24,8 +24,12 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
 
+import static com.android.server.vcn.VcnGatewayConnection.VcnIkeSession;
+import static com.android.server.vcn.VcnGatewayConnection.VcnNetworkAgent;
+
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Matchers.eq;
@@ -191,4 +195,23 @@ public class VcnGatewayConnectionTest extends VcnGatewayConnectionTestBase {
 
         verify(mDisconnectRequestAlarm).cancel();
     }
+
+    @Test
+    public void testQuittingCleansUpPersistentState() {
+        final VcnIkeSession vcnIkeSession = mock(VcnIkeSession.class);
+        final VcnNetworkAgent vcnNetworkAgent = mock(VcnNetworkAgent.class);
+
+        mGatewayConnection.setIkeSession(vcnIkeSession);
+        mGatewayConnection.setNetworkAgent(vcnNetworkAgent);
+
+        mGatewayConnection.quitNow();
+        mTestLooper.dispatchAll();
+
+        assertNull(mGatewayConnection.getIkeSession());
+        verify(vcnIkeSession).kill();
+        assertNull(mGatewayConnection.getNetworkAgent());
+        verify(vcnNetworkAgent).unregister();
+
+        verifyWakeLockReleased();
+    }
 }