Stop MdnsServiceTypeClient send on socket destroy

MdnsServiceTypeClient should stop sending when it is removed due to its
socket being destroyed.

On null networks (downstream interfaces) that may have multiple sockets,
this should only happen once the last socket used by the (null) network
has been destroyed.

Bug: 278635632
Test: atest
(cherry picked from https://android-review.googlesource.com/q/commit:3d66b0432d0f83249d18ffaa757103129359d115)
Merged-In: Ie1808840bd68678f2af7b71bdd8f3be377c14424
Change-Id: Ie1808840bd68678f2af7b71bdd8f3be377c14424

service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java

@@ -154,13 +154,13 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
                     }
 
                     @Override
-                    public void onSocketDestroyed(@Nullable Network network) {
+                    public void onAllSocketsDestroyed(@Nullable Network network) {
                         synchronized (MdnsDiscoveryManager.this) {
                             final MdnsServiceTypeClient serviceTypeClient =
                                     perNetworkServiceTypeClients.get(serviceType, network);
                             if (serviceTypeClient == null) return;
                             // Notify all listeners that all services are removed from this socket.
-                            serviceTypeClient.notifyAllServicesRemoved();
+                            serviceTypeClient.notifySocketDestroyed();
                             perNetworkServiceTypeClients.remove(serviceTypeClient);
                         }
                     }

service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java

@@ -96,7 +96,9 @@ public class MdnsMultinetworkSocketClient implements MdnsSocketClientBase {
 
         private void notifySocketDestroyed(@NonNull MdnsInterfaceSocket socket) {
             final Network network = mActiveNetworkSockets.remove(socket);
-            mSocketCreationCallback.onSocketDestroyed(network);
+            if (!isAnySocketActive(network)) {
+                mSocketCreationCallback.onAllSocketsDestroyed(network);
+            }
         }
 
         void onNetworkUnrequested() {
@@ -119,6 +121,16 @@ public class MdnsMultinetworkSocketClient implements MdnsSocketClientBase {
         return false;
     }
 
+    private boolean isAnySocketActive(@Nullable Network network) {
+        for (int i = 0; i < mRequestedNetworks.size(); i++) {
+            final InterfaceSocketCallback isc = mRequestedNetworks.valueAt(i);
+            if (isc.mActiveNetworkSockets.containsValue(network)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private ArrayMap<MdnsInterfaceSocket, Network> getActiveSockets() {
         final ArrayMap<MdnsInterfaceSocket, Network> sockets = new ArrayMap<>();
         for (int i = 0; i < mRequestedNetworks.size(); i++) {
@@ -182,13 +194,15 @@ public class MdnsMultinetworkSocketClient implements MdnsSocketClientBase {
     @Override
     public void notifyNetworkUnrequested(@NonNull MdnsServiceBrowserListener listener) {
         ensureRunningOnHandlerThread(mHandler);
-        final InterfaceSocketCallback callback = mRequestedNetworks.remove(listener);
+        final InterfaceSocketCallback callback = mRequestedNetworks.get(listener);
         if (callback == null) {
             Log.e(TAG, "Can not be unrequested with unknown listener=" + listener);
             return;
         }
-        mSocketProvider.unrequestSocket(callback);
         callback.onNetworkUnrequested();
+        // onNetworkUnrequested does cleanups based on mRequestedNetworks, only remove afterwards
+        mRequestedNetworks.remove(listener);
+        mSocketProvider.unrequestSocket(callback);
     }
 
     private void sendMdnsPacket(@NonNull DatagramPacket packet, @Nullable Network targetNetwork) {

service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java

@@ -284,7 +284,7 @@ public class MdnsServiceTypeClient {
     }
 
     /** Notify all services are removed because the socket is destroyed. */
-    public void notifyAllServicesRemoved() {
+    public void notifySocketDestroyed() {
         synchronized (lock) {
             for (MdnsResponse response : instanceNameToResponse.values()) {
                 final String name = response.getServiceInstanceName();
@@ -302,6 +302,11 @@ public class MdnsServiceTypeClient {
                     listener.onServiceNameRemoved(serviceInfo);
                 }
             }
+
+            if (requestTaskFuture != null) {
+                requestTaskFuture.cancel(true);
+                requestTaskFuture = null;
+            }
         }
     }
 

service-t/src/com/android/server/connectivity/mdns/MdnsSocketClientBase.java

@@ -88,6 +88,6 @@ public interface MdnsSocketClientBase {
         void onSocketCreated(@Nullable Network network);
 
         /*** Notify requested socket is destroyed */
-        void onSocketDestroyed(@Nullable Network network);
+        void onAllSocketsDestroyed(@Nullable Network network);
     }
 }

tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java

@@ -222,8 +222,8 @@ public class MdnsDiscoveryManagerTests {
         // The client for NETWORK_1 receives the callback that the NETWORK_1 has been destroyed,
         // mockServiceTypeClientOne1 should send service removed notifications and remove from the
         // list of clients.
-        callback.onSocketDestroyed(NETWORK_1);
+        callback.onAllSocketsDestroyed(NETWORK_1);
-        verify(mockServiceTypeClientOne1).notifyAllServicesRemoved();
+        verify(mockServiceTypeClientOne1).notifySocketDestroyed();
 
         // Receive a response again, it should be processed only on mockServiceTypeClientTwo2.
         // Because the mockServiceTypeClientOne1 is removed from the list of clients, it is no
@@ -236,8 +236,8 @@ public class MdnsDiscoveryManagerTests {
 
         // The client for NETWORK_2 receives the callback that the NETWORK_1 has been destroyed,
         // mockServiceTypeClientTwo2 shouldn't send any notifications.
-        callback2.onSocketDestroyed(NETWORK_1);
+        callback2.onAllSocketsDestroyed(NETWORK_1);
-        verify(mockServiceTypeClientTwo2, never()).notifyAllServicesRemoved();
+        verify(mockServiceTypeClientTwo2, never()).notifySocketDestroyed();
 
         // Receive a response again, mockServiceTypeClientTwo2 is still in the list of clients, it's
         // still able to process responses.

tests/unit/java/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClientTest.java

@@ -25,9 +25,11 @@ import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import android.net.InetAddresses;
 import android.net.Network;
@@ -249,10 +251,30 @@ public class MdnsMultinetworkSocketClientTest {
         callback.onSocketCreated(null /* network */, otherSocket, List.of());
         verify(mSocketCreationCallback, times(2)).onSocketCreated(null);
 
+        verify(mSocketCreationCallback, never()).onAllSocketsDestroyed(null /* network */);
         mHandler.post(() -> mSocketClient.notifyNetworkUnrequested(mListener));
         HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
 
         verify(mProvider).unrequestSocket(callback);
-        verify(mSocketCreationCallback, times(2)).onSocketDestroyed(null /* network */);
+        verify(mSocketCreationCallback).onAllSocketsDestroyed(null /* network */);
+    }
+
+    @Test
+    public void testSocketCreatedAndDestroyed_NullNetwork() throws IOException {
+        final MdnsInterfaceSocket otherSocket = mock(MdnsInterfaceSocket.class);
+        final SocketCallback callback = expectSocketCallback(mListener, null /* network */);
+        doReturn(createEmptyNetworkInterface()).when(mSocket).getInterface();
+        doReturn(createEmptyNetworkInterface()).when(otherSocket).getInterface();
+
+        callback.onSocketCreated(null /* network */, mSocket, List.of());
+        verify(mSocketCreationCallback).onSocketCreated(null);
+        callback.onSocketCreated(null /* network */, otherSocket, List.of());
+        verify(mSocketCreationCallback, times(2)).onSocketCreated(null);
+
+        // Notify socket destroyed
+        callback.onInterfaceDestroyed(null /* network */, mSocket);
+        verifyNoMoreInteractions(mSocketCreationCallback);
+        callback.onInterfaceDestroyed(null /* network */, otherSocket);
+        verify(mSocketCreationCallback).onAllSocketsDestroyed(null /* network */);
     }
 }

tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java

@@ -90,6 +90,7 @@ public class MdnsServiceTypeClientTests {
 
     private static final long TEST_TTL = 120000L;
     private static final long TEST_ELAPSED_REALTIME = 123L;
+    private static final long TEST_TIMEOUT_MS = 10_000L;
 
     @Mock
     private MdnsServiceBrowserListener mockListenerOne;
@@ -1169,7 +1170,7 @@ public class MdnsServiceTypeClientTests {
     }
 
     @Test
-    public void testNotifyAllServicesRemoved() {
+    public void testNotifySocketDestroyed() throws Exception {
         client = new MdnsServiceTypeClient(
                 SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork, mockSharedLog);
 
@@ -1182,8 +1183,18 @@ public class MdnsServiceTypeClientTests {
                 .setResolveInstanceName("Instance1").build();
 
         client.startSendAndReceive(mockListenerOne, resolveOptions);
+        // Ensure the first task is executed so it schedules a future task
+        currentThreadExecutor.getAndClearSubmittedFuture().get(
+                TEST_TIMEOUT_MS, TimeUnit.MILLISECONDS);
         client.startSendAndReceive(mockListenerTwo, MdnsSearchOptions.getDefaultOptions());
 
+        // Filing the second request cancels the first future
+        verify(expectedSendFutures[0]).cancel(true);
+
+        // Ensure it gets executed too
+        currentThreadExecutor.getAndClearSubmittedFuture().get(
+                TEST_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+
         // Complete response from instanceName
         client.processResponse(createResponse(
                         requestedInstance, ipV4Address, 5353, SERVICE_TYPE_LABELS,
@@ -1196,7 +1207,9 @@ public class MdnsServiceTypeClientTests {
                         Collections.emptyMap() /* textAttributes */, TEST_TTL),
                 INTERFACE_INDEX, mockNetwork);
 
-        client.notifyAllServicesRemoved();
+        verify(expectedSendFutures[1], never()).cancel(true);
+        client.notifySocketDestroyed();
+        verify(expectedSendFutures[1]).cancel(true);
 
         // mockListenerOne gets notified for the requested instance
         final InOrder inOrder1 = inOrder(mockListenerOne);
@@ -1261,6 +1274,7 @@ public class MdnsServiceTypeClientTests {
         private long lastScheduledDelayInMs;
         private Runnable lastScheduledRunnable;
         private Runnable lastSubmittedRunnable;
+        private Future<?> lastSubmittedFuture;
         private int futureIndex;
 
         FakeExecutor() {
@@ -1272,6 +1286,7 @@ public class MdnsServiceTypeClientTests {
         public Future<?> submit(Runnable command) {
             Future<?> future = super.submit(command);
             lastSubmittedRunnable = command;
+            lastSubmittedFuture = future;
             return future;
         }
 
@@ -1303,6 +1318,12 @@ public class MdnsServiceTypeClientTests {
             lastSubmittedRunnable = null;
             return val;
         }
+
+        Future<?> getAndClearSubmittedFuture() {
+            Future<?> val = lastSubmittedFuture;
+            lastSubmittedFuture = null;
+            return val;
+        }
     }
 
     private MdnsPacket createResponse(