From b63e7aebee91d3e8688daf972332bd72b7e8fcc4 Mon Sep 17 00:00:00 2001
From: Pavan Kumar M <quic_rpavan@quicinc.com>
Date: Thu, 6 Jan 2022 14:19:48 +0530
Subject: [PATCH] Synchronize the IpClient events

If the ipClient is stopped before handling the events in
Handler thread, ethernet network factory might end up
creating a network agent for a network which is already
cleared.

This change fixes the issue by handling the events only
if ipClient is initialized.

Tests:

  Builds, Boots, EthernetNetworkFactoryTest.
  Ethernet related test scenarios

Bug: 207057937
Change-Id: If7ff7abf5f0dcdb0e94de0502bfdf981f9f20298
---
 .../ethernet/EthernetNetworkFactory.java      | 16 ++++++
 .../ethernet/EthernetNetworkFactoryTest.java  | 53 +++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
index b556f6c949..7b727a3ce3 100644
--- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
+++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
@@ -481,6 +481,10 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         void onIpLayerStarted(LinkProperties linkProperties) {
+            if(mIpClient == null) {
+                if (DBG) Log.d(TAG, "IpClient is not initialized.");
+                return;
+            }
             if (mNetworkAgent != null) {
                 Log.e(TAG, "Already have a NetworkAgent - aborting new request");
                 stop();
@@ -528,6 +532,10 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         void onIpLayerStopped(LinkProperties linkProperties) {
+            if(mIpClient == null) {
+                if (DBG) Log.d(TAG, "IpClient is not initialized.");
+                return;
+            }
             // This cannot happen due to provisioning timeout, because our timeout is 0. It can only
             // happen if we're provisioned and we lose provisioning.
             stop();
@@ -539,6 +547,10 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         void updateLinkProperties(LinkProperties linkProperties) {
+            if(mIpClient == null) {
+                if (DBG) Log.d(TAG, "IpClient is not initialized.");
+                return;
+            }
             mLinkProperties = linkProperties;
             if (mNetworkAgent != null) {
                 mNetworkAgent.sendLinkPropertiesImpl(linkProperties);
@@ -546,6 +558,10 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         void updateNeighborLostEvent(String logMsg) {
+            if(mIpClient == null) {
+                if (DBG) Log.d(TAG, "IpClient is not initialized.");
+                return;
+            }
             Log.i(TAG, "updateNeighborLostEvent " + logMsg);
             // Reachability lost will be seen only if the gateway is not reachable.
             // Since ethernet FW doesn't have the mechanism to scan for new networks
diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
index 44ed26c9b4..52ddf3c8ad 100644
--- a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
+++ b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
@@ -469,6 +469,59 @@ public class EthernetNetworkFactoryTest {
         verifyRestart(createDefaultIpConfig());
     }
 
+    @Test
+    public void testIgnoreOnIpLayerStartedCallbackAfterIpClientHasStopped() throws Exception {
+        createAndVerifyProvisionedInterface(TEST_IFACE);
+        mIpClientCallbacks.onProvisioningFailure(new LinkProperties());
+        mIpClientCallbacks.onProvisioningSuccess(new LinkProperties());
+        mLooper.dispatchAll();
+        verifyStop();
+
+        // ipClient has been shut down first, we should not retry
+        verify(mIpClient, never()).startProvisioning(any());
+        verify(mNetworkAgent, never()).register();
+    }
+
+    @Test
+    public void testIgnoreOnIpLayerStoppedCallbackAfterIpClientHasStopped() throws Exception {
+        createAndVerifyProvisionedInterface(TEST_IFACE);
+        when(mDeps.getNetworkInterfaceByName(TEST_IFACE)).thenReturn(mInterfaceParams);
+        mIpClientCallbacks.onProvisioningFailure(new LinkProperties());
+        mIpClientCallbacks.onProvisioningFailure(new LinkProperties());
+        mLooper.dispatchAll();
+        verifyStop();
+
+        // ipClient has been shut down first, we should not retry
+        verify(mIpClient).startProvisioning(any());
+    }
+
+    @Test
+    public void testIgnoreLinkPropertiesCallbackAfterIpClientHasStopped() throws Exception {
+        createAndVerifyProvisionedInterface(TEST_IFACE);
+        LinkProperties lp = new LinkProperties();
+
+        mIpClientCallbacks.onProvisioningFailure(lp);
+        mIpClientCallbacks.onLinkPropertiesChange(lp);
+        mLooper.dispatchAll();
+        verifyStop();
+
+        // ipClient has been shut down first, we should not update
+        verify(mNetworkAgent, never()).sendLinkPropertiesImpl(same(lp));
+    }
+
+    @Test
+    public void testIgnoreNeighborLossCallbackAfterIpClientHasStopped() throws Exception {
+        createAndVerifyProvisionedInterface(TEST_IFACE);
+        mIpClientCallbacks.onProvisioningFailure(new LinkProperties());
+        mIpClientCallbacks.onReachabilityLost("Neighbor Lost");
+        mLooper.dispatchAll();
+        verifyStop();
+
+        // ipClient has been shut down first, we should not update
+        verify(mIpClient, never()).startProvisioning(any());
+        verify(mNetworkAgent, never()).register();
+    }
+
     private void verifyRestart(@NonNull final IpConfiguration ipConfig) {
         verifyStop();
         verifyStart(ipConfig);