Remove the ServiceTypeClient after socket destroyed
The ServiceTypeClient should be removed after socket destroyed
because it's no longer used by any request. The requests will be
matched to the newly created ServiceTypeClient.
Bug: 278635632
Test: atest FrameworksNetTests CtsNetTestCases
Change-Id: Ia917b14d3666f3bfe8e874606a34800a4ce65c5a
diff --git a/framework-t/src/android/net/nsd/NsdManager.java b/framework-t/src/android/net/nsd/NsdManager.java
index d119db6..2930cbd 100644
--- a/framework-t/src/android/net/nsd/NsdManager.java
+++ b/framework-t/src/android/net/nsd/NsdManager.java
@@ -429,6 +429,10 @@
private final DiscoveryListener mWrapped;
private final Executor mWrappedExecutor;
private final ArraySet<TrackedNsdInfo> mFoundInfo = new ArraySet<>();
+ // When this flag is set to true, no further service found or lost callbacks should be
+ // handled. This flag indicates that the network for this DelegatingDiscoveryListener is
+ // lost, and any further callbacks would be redundant.
+ private boolean mAllServicesLost = false;
private DelegatingDiscoveryListener(Network network, DiscoveryListener listener,
Executor executor) {
@@ -445,6 +449,7 @@
serviceInfo.setNetwork(mNetwork);
mWrappedExecutor.execute(() -> mWrapped.onServiceLost(serviceInfo));
}
+ mAllServicesLost = true;
}
@Override
@@ -486,12 +491,22 @@
@Override
public void onServiceFound(NsdServiceInfo serviceInfo) {
+ if (mAllServicesLost) {
+ // This DelegatingDiscoveryListener no longer has a network connection. Ignore
+ // the callback.
+ return;
+ }
mFoundInfo.add(new TrackedNsdInfo(serviceInfo));
mWrappedExecutor.execute(() -> mWrapped.onServiceFound(serviceInfo));
}
@Override
public void onServiceLost(NsdServiceInfo serviceInfo) {
+ if (mAllServicesLost) {
+ // This DelegatingDiscoveryListener no longer has a network connection. Ignore
+ // the callback.
+ return;
+ }
mFoundInfo.remove(new TrackedNsdInfo(serviceInfo));
mWrappedExecutor.execute(() -> mWrapped.onServiceLost(serviceInfo));
}
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 849eac1..6455044 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
@@ -135,18 +135,36 @@
}
}
// Request the network for discovery.
- socketClient.notifyNetworkRequested(listener, searchOptions.getNetwork(), network -> {
- synchronized (this) {
- // All listeners of the same service types shares the same MdnsServiceTypeClient.
- MdnsServiceTypeClient serviceTypeClient =
- perNetworkServiceTypeClients.get(serviceType, network);
- if (serviceTypeClient == null) {
- serviceTypeClient = createServiceTypeClient(serviceType, network);
- perNetworkServiceTypeClients.put(serviceType, network, serviceTypeClient);
- }
- serviceTypeClient.startSendAndReceive(listener, searchOptions);
- }
- });
+ socketClient.notifyNetworkRequested(listener, searchOptions.getNetwork(),
+ new MdnsSocketClientBase.SocketCreationCallback() {
+ @Override
+ public void onSocketCreated(@Nullable Network network) {
+ synchronized (MdnsDiscoveryManager.this) {
+ // All listeners of the same service types shares the same
+ // MdnsServiceTypeClient.
+ MdnsServiceTypeClient serviceTypeClient =
+ perNetworkServiceTypeClients.get(serviceType, network);
+ if (serviceTypeClient == null) {
+ serviceTypeClient = createServiceTypeClient(serviceType, network);
+ perNetworkServiceTypeClients.put(serviceType, network,
+ serviceTypeClient);
+ }
+ serviceTypeClient.startSendAndReceive(listener, searchOptions);
+ }
+ }
+
+ @Override
+ public void onSocketDestroyed(@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();
+ 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 4504bb6..6414453 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java
@@ -90,6 +90,7 @@
@NonNull MdnsInterfaceSocket socket) {
mSocketPacketHandlers.remove(socket);
mActiveNetworkSockets.remove(socket);
+ mSocketCreationCallback.onSocketDestroyed(network);
}
}
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 6585d44..35f9b04 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java
@@ -263,6 +263,28 @@
}
}
+ /** Notify all services are removed because the socket is destroyed. */
+ public void notifyAllServicesRemoved() {
+ synchronized (lock) {
+ for (MdnsResponse response : instanceNameToResponse.values()) {
+ final String name = response.getServiceInstanceName();
+ if (name == null) continue;
+ for (int i = 0; i < listeners.size(); i++) {
+ if (!responseMatchesOptions(response, listeners.valueAt(i))) continue;
+ final MdnsServiceBrowserListener listener = listeners.keyAt(i);
+ final MdnsServiceInfo serviceInfo =
+ buildMdnsServiceInfoFromResponse(response, serviceTypeLabels);
+ if (response.isComplete()) {
+ sharedLog.log("Socket destroyed. onServiceRemoved: " + name);
+ listener.onServiceRemoved(serviceInfo);
+ }
+ sharedLog.log("Socket destroyed. onServiceNameRemoved: " + name);
+ listener.onServiceNameRemoved(serviceInfo);
+ }
+ }
+ }
+ }
+
private void onResponseModified(@NonNull MdnsResponse response) {
final String serviceInstanceName = response.getServiceInstanceName();
final MdnsResponse currentResponse =
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 ebafc49..6bcad01 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClientBase.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClientBase.java
@@ -86,5 +86,8 @@
interface SocketCreationCallback {
/*** Notify requested socket is created */
void onSocketCreated(@Nullable Network network);
+
+ /*** Notify requested socket is destroyed */
+ void onSocketDestroyed(@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 0a23ba5..63357f1 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java
@@ -20,6 +20,7 @@
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -193,6 +194,60 @@
verify(mockServiceTypeClientTwo2).processResponse(responseForSubtype, ifIndex, NETWORK_2);
}
+ @Test
+ public void testSocketCreatedAndDestroyed() throws IOException {
+ // Create a ServiceTypeClient for SERVICE_TYPE_1 and NETWORK_1
+ final MdnsSearchOptions options1 =
+ MdnsSearchOptions.newBuilder().setNetwork(NETWORK_1).build();
+ final SocketCreationCallback callback = expectSocketCreationCallback(
+ SERVICE_TYPE_1, mockListenerOne, options1);
+ callback.onSocketCreated(NETWORK_1);
+ verify(mockServiceTypeClientOne1).startSendAndReceive(mockListenerOne, options1);
+
+ // Create a ServiceTypeClient for SERVICE_TYPE_2 and NETWORK_2
+ final MdnsSearchOptions options2 =
+ MdnsSearchOptions.newBuilder().setNetwork(NETWORK_2).build();
+ final SocketCreationCallback callback2 = expectSocketCreationCallback(
+ SERVICE_TYPE_2, mockListenerTwo, options2);
+ callback2.onSocketCreated(NETWORK_2);
+ verify(mockServiceTypeClientTwo2).startSendAndReceive(mockListenerTwo, options2);
+
+ // Receive a response, it should be processed on both clients.
+ final MdnsPacket response = createMdnsPacket(SERVICE_TYPE_1);
+ final int ifIndex = 1;
+ discoveryManager.onResponseReceived(response, ifIndex, null /* network */);
+ verify(mockServiceTypeClientOne1).processResponse(response, ifIndex, null /* network */);
+ verify(mockServiceTypeClientTwo2).processResponse(response, ifIndex, null /* network */);
+
+ // 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();
+
+ // Receive a response again, it should be processed only on mockServiceTypeClientTwo2.
+ // Because the mockServiceTypeClientOne1 is removed from the list of clients, it is no
+ // longer able to process responses.
+ discoveryManager.onResponseReceived(response, ifIndex, null /* network */);
+ verify(mockServiceTypeClientOne1, times(1))
+ .processResponse(response, ifIndex, null /* network */);
+ verify(mockServiceTypeClientTwo2, times(2))
+ .processResponse(response, ifIndex, null /* network */);
+
+ // 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();
+
+ // Receive a response again, mockServiceTypeClientTwo2 is still in the list of clients, it's
+ // still able to process responses.
+ discoveryManager.onResponseReceived(response, ifIndex, null /* network */);
+ verify(mockServiceTypeClientOne1, times(1))
+ .processResponse(response, ifIndex, null /* network */);
+ verify(mockServiceTypeClientTwo2, times(3))
+ .processResponse(response, ifIndex, null /* network */);
+ }
+
private MdnsPacket createMdnsPacket(String serviceType) {
final String[] type = TextUtils.split(serviceType, "\\.");
final ArrayList<String> name = new ArrayList<>(type.length + 1);
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 34b44fc..2fcdff2 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java
@@ -1056,6 +1056,61 @@
inOrder.verify(mockListenerTwo).onServiceRemoved(matchServiceName(otherInstance));
}
+ @Test
+ public void testNotifyAllServicesRemoved() {
+ client = new MdnsServiceTypeClient(
+ SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork, mockSharedLog);
+
+ final String requestedInstance = "instance1";
+ final String otherInstance = "instance2";
+ final String ipV4Address = "192.0.2.0";
+
+ final MdnsSearchOptions resolveOptions = MdnsSearchOptions.newBuilder()
+ // Use different case in the options
+ .setResolveInstanceName("Instance1").build();
+
+ client.startSendAndReceive(mockListenerOne, resolveOptions);
+ client.startSendAndReceive(mockListenerTwo, MdnsSearchOptions.getDefaultOptions());
+
+ // Complete response from instanceName
+ client.processResponse(createResponse(
+ requestedInstance, ipV4Address, 5353, SERVICE_TYPE_LABELS,
+ Collections.emptyMap() /* textAttributes */, TEST_TTL),
+ INTERFACE_INDEX, mockNetwork);
+
+ // Complete response from otherInstanceName
+ client.processResponse(createResponse(
+ otherInstance, ipV4Address, 5353, SERVICE_TYPE_LABELS,
+ Collections.emptyMap() /* textAttributes */, TEST_TTL),
+ INTERFACE_INDEX, mockNetwork);
+
+ client.notifyAllServicesRemoved();
+
+ // mockListenerOne gets notified for the requested instance
+ final InOrder inOrder1 = inOrder(mockListenerOne);
+ inOrder1.verify(mockListenerOne).onServiceNameDiscovered(
+ matchServiceName(requestedInstance));
+ inOrder1.verify(mockListenerOne).onServiceFound(matchServiceName(requestedInstance));
+ inOrder1.verify(mockListenerOne).onServiceRemoved(matchServiceName(requestedInstance));
+ inOrder1.verify(mockListenerOne).onServiceNameRemoved(matchServiceName(requestedInstance));
+ verify(mockListenerOne, never()).onServiceFound(matchServiceName(otherInstance));
+ verify(mockListenerOne, never()).onServiceNameDiscovered(matchServiceName(otherInstance));
+ verify(mockListenerOne, never()).onServiceRemoved(matchServiceName(otherInstance));
+ verify(mockListenerOne, never()).onServiceNameRemoved(matchServiceName(otherInstance));
+
+ // mockListenerTwo gets notified for both though
+ final InOrder inOrder2 = inOrder(mockListenerTwo);
+ inOrder2.verify(mockListenerTwo).onServiceNameDiscovered(
+ matchServiceName(requestedInstance));
+ inOrder2.verify(mockListenerTwo).onServiceFound(matchServiceName(requestedInstance));
+ inOrder2.verify(mockListenerTwo).onServiceNameDiscovered(matchServiceName(otherInstance));
+ inOrder2.verify(mockListenerTwo).onServiceFound(matchServiceName(otherInstance));
+ inOrder2.verify(mockListenerTwo).onServiceRemoved(matchServiceName(otherInstance));
+ inOrder2.verify(mockListenerTwo).onServiceNameRemoved(matchServiceName(otherInstance));
+ inOrder2.verify(mockListenerTwo).onServiceRemoved(matchServiceName(requestedInstance));
+ inOrder2.verify(mockListenerTwo).onServiceNameRemoved(matchServiceName(requestedInstance));
+ }
+
private static MdnsServiceInfo matchServiceName(String name) {
return argThat(info -> info.getServiceInstanceName().equals(name));
}