Merge "Always stop dhcp server even it is obsolete" am: 0ae446b50d am: d2ca7e8b1e

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1370217

Change-Id: Ibb1c96fce2435fec3f678d89531ed10a6ddc784f

2 files changed, 50 insertions, 14 deletions

diff --git a/packages/Tethering/src/android/net/ip/IpServer.java b/packages/Tethering/src/android/net/ip/IpServer.java
index 8af1797a9dd7..9131317e8d70 100644
--- a/packages/Tethering/src/android/net/ip/IpServer.java
+++ b/packages/Tethering/src/android/net/ip/IpServer.java
@@ -423,9 +423,13 @@ public class IpServer extends StateMachine {
             getHandler().post(() -> {
                 // We are on the handler thread: mDhcpServerStartIndex can be read safely.
                 if (mStartIndex != mDhcpServerStartIndex) {
-                    // This start request is obsolete. When the |server| binder token goes out of
-                    // scope, the garbage collector will finalize it, which causes the network stack
-                    // process garbage collector to collect the server itself.
+                     // This start request is obsolete. Explicitly stop the DHCP server to shut
+                     // down its thread. When the |server| binder token goes out of scope, the
+                     // garbage collector will finalize it, which causes the network stack process
+                     // garbage collector to collect the server itself.
+                    try {
+                        server.stop(null);
+                    } catch (RemoteException e) { }
                     return;
                 }
 
diff --git a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
index 30a9d2252ea6..3b72b5b47191 100644
--- a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
+++ b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
@@ -50,6 +50,7 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.inOrder;
@@ -73,6 +74,7 @@ import android.net.MacAddress;
 import android.net.RouteInfo;
 import android.net.TetherOffloadRuleParcel;
 import android.net.TetherStatsParcel;
+import android.net.dhcp.DhcpServerCallbacks;
 import android.net.dhcp.DhcpServingParamsParcel;
 import android.net.dhcp.IDhcpEventCallbacks;
 import android.net.dhcp.IDhcpServer;
@@ -163,17 +165,6 @@ public class IpServerTest {
 
     private void initStateMachine(int interfaceType, boolean usingLegacyDhcp,
             boolean usingBpfOffload) throws Exception {
-        doAnswer(inv -> {
-            final IDhcpServerCallbacks cb = inv.getArgument(2);
-            new Thread(() -> {
-                try {
-                    cb.onDhcpServerCreated(STATUS_SUCCESS, mDhcpServer);
-                } catch (RemoteException e) {
-                    fail(e.getMessage());
-                }
-            }).run();
-            return null;
-        }).when(mDependencies).makeDhcpServer(any(), mDhcpParamsCaptor.capture(), any());
         when(mDependencies.getRouterAdvertisementDaemon(any())).thenReturn(mRaDaemon);
         when(mDependencies.getInterfaceParams(IFACE_NAME)).thenReturn(TEST_IFACE_PARAMS);
 
@@ -225,6 +216,20 @@ public class IpServerTest {
         when(mAddressCoordinator.requestDownstreamAddress(any())).thenReturn(mTestAddress);
     }
 
+    private void setUpDhcpServer() throws Exception {
+        doAnswer(inv -> {
+            final IDhcpServerCallbacks cb = inv.getArgument(2);
+            new Thread(() -> {
+                try {
+                    cb.onDhcpServerCreated(STATUS_SUCCESS, mDhcpServer);
+                } catch (RemoteException e) {
+                    fail(e.getMessage());
+                }
+            }).run();
+            return null;
+        }).when(mDependencies).makeDhcpServer(any(), mDhcpParamsCaptor.capture(), any());
+    }
+
     @Before public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
         when(mSharedLog.forSubComponent(anyString())).thenReturn(mSharedLog);
@@ -258,6 +263,8 @@ public class IpServerTest {
                         return mTetherConfig;
                     }
                 }));
+
+        setUpDhcpServer();
     }
 
     @Test
@@ -965,6 +972,31 @@ public class IpServerTest {
         reset(mRaDaemon);
     }
 
+    @Test
+    public void testStopObsoleteDhcpServer() throws Exception {
+        final ArgumentCaptor<DhcpServerCallbacks> cbCaptor =
+                ArgumentCaptor.forClass(DhcpServerCallbacks.class);
+        doNothing().when(mDependencies).makeDhcpServer(any(), mDhcpParamsCaptor.capture(),
+                cbCaptor.capture());
+        initStateMachine(TETHERING_WIFI);
+        dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED);
+        verify(mDhcpServer, never()).startWithCallbacks(any(), any());
+
+        // No stop dhcp server because dhcp server is not created yet.
+        dispatchCommand(IpServer.CMD_TETHER_UNREQUESTED);
+        verify(mDhcpServer, never()).stop(any());
+
+        // Stop obsolete dhcp server.
+        try {
+            final DhcpServerCallbacks cb = cbCaptor.getValue();
+            cb.onDhcpServerCreated(STATUS_SUCCESS, mDhcpServer);
+            mLooper.dispatchAll();
+        } catch (RemoteException e) {
+            fail(e.getMessage());
+        }
+        verify(mDhcpServer).stop(any());
+    }
+
     private void assertDhcpServingParams(final DhcpServingParamsParcel params,
             final IpPrefix prefix) {
         // Last address byte is random