LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Stop MdnsServiceTypeClient send on socket destroy

Browse Source 
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
This commit is contained in:
Remi NGUYEN VAN
2023-05-12 16:52:24 +09:00
parent 5fe9bacc63
commit 3d66b0432d
7 changed files with 76 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java
View File

@@ -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);
}
}

20
service-t/src/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClient.java
View File

@@ -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<MdnsInterfaceSocket, Network> getActiveSockets() {
final ArrayMap<MdnsInterfaceSocket, Network> 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) {

7
service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java
View File

@@ -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;
}
}
}

2
service-t/src/com/android/server/connectivity/mdns/MdnsSocketClientBase.java
View File

@@ -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);
}
}

8
tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java
View File

@@ -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.

24
tests/unit/java/com/android/server/connectivity/mdns/MdnsMultinetworkSocketClientTest.java
View File

@@ -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 */);
}
}

25
tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java
View File

@@ -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(