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
Change-Id: Ie1808840bd68678f2af7b71bdd8f3be377c14424
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 6455044..2281c81 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 @@
}
@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 ad8cb64..52c8622 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 @@
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 @@
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 @@
@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 4e6571f..e56bd5b 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 @@
}
/** 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 @@
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 6bcad01..c614cd3 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 @@
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 63357f1..45da874 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 @@
// 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 @@
// 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 d5d2902..c6137c6 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.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 @@
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 bd59156..4e69f47 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 @@
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 @@
}
@Test
- public void testNotifyAllServicesRemoved() {
+ public void testNotifySocketDestroyed() throws Exception {
client = new MdnsServiceTypeClient(
SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork, mockSharedLog);
@@ -1182,8 +1183,18 @@
.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 @@
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 @@
private long lastScheduledDelayInMs;
private Runnable lastScheduledRunnable;
private Runnable lastSubmittedRunnable;
+ private Future<?> lastSubmittedFuture;
private int futureIndex;
FakeExecutor() {
@@ -1272,6 +1286,7 @@
public Future<?> submit(Runnable command) {
Future<?> future = super.submit(command);
lastSubmittedRunnable = command;
+ lastSubmittedFuture = future;
return future;
}
@@ -1303,6 +1318,12 @@
lastSubmittedRunnable = null;
return val;
}
+
+ Future<?> getAndClearSubmittedFuture() {
+ Future<?> val = lastSubmittedFuture;
+ lastSubmittedFuture = null;
+ return val;
+ }
}
private MdnsPacket createResponse(