Merge "Stop MdnsServiceTypeClient send on socket destroy"

This commit is contained in:
Remi NGUYEN VAN
2023-05-16 04:03:42 +00:00
committed by Gerrit Code Review
7 changed files with 76 additions and 14 deletions

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

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) {

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

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

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.

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

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(