Merge changes from topic "cherrypicker-L76200000960680946:N26500001369291245" into udc-dev
* changes:
Stop MdnsServiceTypeClient send on socket destroy
Do not send socket destroyed on unregistration
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 6414453..52c8622 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java
@@ -34,7 +34,6 @@
import java.net.Inet6Address;
import java.net.InetSocketAddress;
import java.util.List;
-import java.util.Map;
/**
* The {@link MdnsMultinetworkSocketClient} manages the multinetwork socket for mDns
@@ -48,9 +47,8 @@
@NonNull private final Handler mHandler;
@NonNull private final MdnsSocketProvider mSocketProvider;
- private final Map<MdnsServiceBrowserListener, InterfaceSocketCallback> mRequestedNetworks =
+ private final ArrayMap<MdnsServiceBrowserListener, InterfaceSocketCallback> mRequestedNetworks =
new ArrayMap<>();
- private final ArrayMap<MdnsInterfaceSocket, Network> mActiveNetworkSockets = new ArrayMap<>();
private final ArrayMap<MdnsInterfaceSocket, ReadPacketHandler> mSocketPacketHandlers =
new ArrayMap<>();
private MdnsSocketClientBase.Callback mCallback = null;
@@ -63,7 +61,11 @@
}
private class InterfaceSocketCallback implements MdnsSocketProvider.SocketCallback {
+ @NonNull
private final SocketCreationCallback mSocketCreationCallback;
+ @NonNull
+ private final ArrayMap<MdnsInterfaceSocket, Network> mActiveNetworkSockets =
+ new ArrayMap<>();
InterfaceSocketCallback(SocketCreationCallback socketCreationCallback) {
mSocketCreationCallback = socketCreationCallback;
@@ -88,10 +90,59 @@
@Override
public void onInterfaceDestroyed(@Nullable Network network,
@NonNull MdnsInterfaceSocket socket) {
- mSocketPacketHandlers.remove(socket);
- mActiveNetworkSockets.remove(socket);
- mSocketCreationCallback.onSocketDestroyed(network);
+ notifySocketDestroyed(socket);
+ maybeCleanupPacketHandler(socket);
}
+
+ private void notifySocketDestroyed(@NonNull MdnsInterfaceSocket socket) {
+ final Network network = mActiveNetworkSockets.remove(socket);
+ if (!isAnySocketActive(network)) {
+ mSocketCreationCallback.onAllSocketsDestroyed(network);
+ }
+ }
+
+ void onNetworkUnrequested() {
+ for (int i = mActiveNetworkSockets.size() - 1; i >= 0; i--) {
+ // Iterate from the end so the socket can be removed
+ final MdnsInterfaceSocket socket = mActiveNetworkSockets.keyAt(i);
+ notifySocketDestroyed(socket);
+ maybeCleanupPacketHandler(socket);
+ }
+ }
+ }
+
+ private boolean isSocketActive(@NonNull MdnsInterfaceSocket socket) {
+ for (int i = 0; i < mRequestedNetworks.size(); i++) {
+ final InterfaceSocketCallback isc = mRequestedNetworks.valueAt(i);
+ if (isc.mActiveNetworkSockets.containsKey(socket)) {
+ return true;
+ }
+ }
+ 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++) {
+ final InterfaceSocketCallback isc = mRequestedNetworks.valueAt(i);
+ sockets.putAll(isc.mActiveNetworkSockets);
+ }
+ return sockets;
+ }
+
+ private void maybeCleanupPacketHandler(@NonNull MdnsInterfaceSocket socket) {
+ if (isSocketActive(socket)) return;
+ mSocketPacketHandlers.remove(socket);
}
private class ReadPacketHandler implements MulticastPacketReader.PacketHandler {
@@ -143,11 +194,14 @@
@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;
}
+ callback.onNetworkUnrequested();
+ // onNetworkUnrequested does cleanups based on mRequestedNetworks, only remove afterwards
+ mRequestedNetworks.remove(listener);
mSocketProvider.unrequestSocket(callback);
}
@@ -156,9 +210,10 @@
instanceof Inet6Address;
final boolean isIpv4 = ((InetSocketAddress) packet.getSocketAddress()).getAddress()
instanceof Inet4Address;
- for (int i = 0; i < mActiveNetworkSockets.size(); i++) {
- final MdnsInterfaceSocket socket = mActiveNetworkSockets.keyAt(i);
- final Network network = mActiveNetworkSockets.valueAt(i);
+ final ArrayMap<MdnsInterfaceSocket, Network> activeSockets = getActiveSockets();
+ for (int i = 0; i < activeSockets.size(); i++) {
+ final MdnsInterfaceSocket socket = activeSockets.keyAt(i);
+ final Network network = activeSockets.valueAt(i);
// Check ip capability and network before sending packet
if (((isIpv6 && socket.hasJoinedIpv6()) || (isIpv4 && socket.hasJoinedIpv4()))
&& isNetworkMatched(targetNetwork, 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 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/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java
index ca61d34..e245ff1 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java
@@ -599,8 +599,6 @@
if (matchRequestedNetwork(network)) continue;
final SocketInfo info = mNetworkSockets.removeAt(i);
info.mSocket.destroy();
- // Still notify to unrequester for socket destroy.
- cb.onInterfaceDestroyed(network, info.mSocket);
mSharedLog.log("Remove socket on net:" + network + " after unrequestSocket");
}
@@ -610,8 +608,6 @@
for (int i = mTetherInterfaceSockets.size() - 1; i >= 0; i--) {
final SocketInfo info = mTetherInterfaceSockets.valueAt(i);
info.mSocket.destroy();
- // Still notify to unrequester for socket destroy.
- cb.onInterfaceDestroyed(null /* network */, info.mSocket);
mSharedLog.log("Remove socket on ifName:" + mTetherInterfaceSockets.keyAt(i)
+ " after unrequestSocket");
}
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 90c43e5..c6137c6 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClientTest.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClientTest.java
@@ -24,8 +24,12 @@
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;
@@ -77,12 +81,17 @@
}
private SocketCallback expectSocketCallback() {
+ return expectSocketCallback(mListener, mNetwork);
+ }
+
+ private SocketCallback expectSocketCallback(MdnsServiceBrowserListener listener,
+ Network requestedNetwork) {
final ArgumentCaptor<SocketCallback> callbackCaptor =
ArgumentCaptor.forClass(SocketCallback.class);
mHandler.post(() -> mSocketClient.notifyNetworkRequested(
- mListener, mNetwork, mSocketCreationCallback));
+ listener, requestedNetwork, mSocketCreationCallback));
verify(mProvider, timeout(DEFAULT_TIMEOUT))
- .requestSocket(eq(mNetwork), callbackCaptor.capture());
+ .requestSocket(eq(requestedNetwork), callbackCaptor.capture());
return callbackCaptor.getValue();
}
@@ -169,4 +178,103 @@
new String[] { "Android", "local" } /* serviceHost */)
), response.answers);
}
+
+ @Test
+ public void testSocketRemovedAfterNetworkUnrequested() throws IOException {
+ // Request a socket
+ final SocketCallback callback = expectSocketCallback(mListener, mNetwork);
+ final DatagramPacket ipv4Packet = new DatagramPacket(BUFFER, 0 /* offset */, BUFFER.length,
+ InetAddresses.parseNumericAddress("192.0.2.1"), 0 /* port */);
+ doReturn(true).when(mSocket).hasJoinedIpv4();
+ doReturn(true).when(mSocket).hasJoinedIpv6();
+ doReturn(createEmptyNetworkInterface()).when(mSocket).getInterface();
+ // Notify socket created
+ callback.onSocketCreated(mNetwork, mSocket, List.of());
+ verify(mSocketCreationCallback).onSocketCreated(mNetwork);
+
+ // Send IPv4 packet and verify sending has been called.
+ mSocketClient.sendMulticastPacket(ipv4Packet);
+ HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
+ verify(mSocket).send(ipv4Packet);
+
+ // Request another socket with null network
+ final MdnsServiceBrowserListener listener2 = mock(MdnsServiceBrowserListener.class);
+ final Network network2 = mock(Network.class);
+ final MdnsInterfaceSocket socket2 = mock(MdnsInterfaceSocket.class);
+ final SocketCallback callback2 = expectSocketCallback(listener2, null);
+ doReturn(true).when(socket2).hasJoinedIpv4();
+ doReturn(true).when(socket2).hasJoinedIpv6();
+ doReturn(createEmptyNetworkInterface()).when(socket2).getInterface();
+ // Notify socket created for two networks.
+ callback2.onSocketCreated(mNetwork, mSocket, List.of());
+ callback2.onSocketCreated(network2, socket2, List.of());
+ verify(mSocketCreationCallback, times(2)).onSocketCreated(mNetwork);
+ verify(mSocketCreationCallback).onSocketCreated(network2);
+
+ // Send IPv4 packet and verify sending to two sockets.
+ mSocketClient.sendMulticastPacket(ipv4Packet);
+ HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
+ verify(mSocket, times(2)).send(ipv4Packet);
+ verify(socket2).send(ipv4Packet);
+
+ // Unrequest another socket
+ mHandler.post(() -> mSocketClient.notifyNetworkUnrequested(listener2));
+ verify(mProvider, timeout(DEFAULT_TIMEOUT)).unrequestSocket(callback2);
+
+ // Send IPv4 packet again and verify only sending via mSocket
+ mSocketClient.sendMulticastPacket(ipv4Packet);
+ HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
+ verify(mSocket, times(3)).send(ipv4Packet);
+ verify(socket2).send(ipv4Packet);
+
+ // Unrequest remaining socket
+ mHandler.post(() -> mSocketClient.notifyNetworkUnrequested(mListener));
+ verify(mProvider, timeout(DEFAULT_TIMEOUT)).unrequestSocket(callback);
+
+ // Send IPv4 packet and verify no more sending.
+ mSocketClient.sendMulticastPacket(ipv4Packet);
+ HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
+ verify(mSocket, times(3)).send(ipv4Packet);
+ verify(socket2).send(ipv4Packet);
+ }
+
+ @Test
+ public void testNotifyNetworkUnrequested_SocketsOnNullNetwork() {
+ final MdnsInterfaceSocket otherSocket = mock(MdnsInterfaceSocket.class);
+ final SocketCallback callback = expectSocketCallback(
+ mListener, null /* requestedNetwork */);
+ 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);
+
+ verify(mSocketCreationCallback, never()).onAllSocketsDestroyed(null /* network */);
+ mHandler.post(() -> mSocketClient.notifyNetworkUnrequested(mListener));
+ HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
+
+ verify(mProvider).unrequestSocket(callback);
+ 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(
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java
index 744ec84..4b87556 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java
@@ -349,8 +349,8 @@
HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
testCallback1.expectedNoCallback();
testCallback2.expectedNoCallback();
- // Expect the socket destroy for tethered interface.
- testCallback3.expectedInterfaceDestroyedForNetwork(null /* network */);
+ // There was still a tethered interface, but no callback should be sent once unregistered
+ testCallback3.expectedNoCallback();
}
private RtNetlinkAddressMessage createNetworkAddressUpdateNetLink(
@@ -528,7 +528,8 @@
HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
mHandler.post(()-> mSocketProvider.unrequestSocket(testCallback));
HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT);
- testCallback.expectedInterfaceDestroyedForNetwork(TEST_NETWORK);
+ // No callback sent when unregistered
+ testCallback.expectedNoCallback();
verify(mCm, times(1)).unregisterNetworkCallback(any(NetworkCallback.class));
verify(mTm, times(1)).unregisterTetheringEventCallback(any());