Merge "Stop MdnsServiceTypeClient send on socket destroy" am: 3623306ccd
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2585400
Change-Id: I529e4eedb5b912ef2a1dc916ab8eaa83c0c306a5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
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(