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

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

Change-Id: I6eb68ffd14d3491784d643e0059ca6591de7d8a6

Tethering/src/android/net/ip/IpServer.java

@@ -422,9 +422,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;
                 }
 

Tethering/tests/unit/src/android/net/ip/IpServerTest.java

@@ -49,6 +49,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;
@@ -72,6 +73,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;
@@ -162,17 +164,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);
 
@@ -224,6 +215,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);
@@ -257,6 +262,8 @@ public class IpServerTest {
                         return mTetherConfig;
                     }
                 }));
+
+        setUpDhcpServer();
     }
 
     @Test
@@ -964,6 +971,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