diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java index 6455044a13..2281c81f75 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java +++ b/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); } } diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java b/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java index ad8cb6464c..52c86228e2 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java +++ b/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 getActiveSockets() { final ArrayMap 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) { diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java index 4e6571fd42..e56bd5b993 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java +++ b/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; + } } } diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClientBase.java b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClientBase.java index 6bcad01a82..c614cd3d18 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClientBase.java +++ b/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); } } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java index 63357f1241..45da874139 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java +++ b/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); - verify(mockServiceTypeClientOne1).notifyAllServicesRemoved(); + callback.onAllSocketsDestroyed(NETWORK_1); + 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); - verify(mockServiceTypeClientTwo2, never()).notifyAllServicesRemoved(); + callback2.onAllSocketsDestroyed(NETWORK_1); + verify(mockServiceTypeClientTwo2, never()).notifySocketDestroyed(); // Receive a response again, mockServiceTypeClientTwo2 is still in the list of clients, it's // still able to process responses. diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClientTest.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClientTest.java index d5d2902c97..c6137c61a3 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClientTest.java +++ b/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 */); } } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java index bd591569b0..4e69f473e0 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java +++ b/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(