Merge "Stop MdnsServiceTypeClient send on socket destroy"
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 */);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user