Share NetworkInterfaceProvider between MdnsSockets
Instead of creating one NetworkInterfaceProvider for each MdnsSocket,
create one in MdnsSocketClient and use it for each MdnsSocket.
MdnsSocket only calls getMulticastNetworkInterfaces and
getAvailableNetwork on MulticastNetworkInterfaceProvider, which are
thread-safe (the first one is synchronized). There is no need to have
multiple MulticastNetworkInterfaceProvider tracking the same state.
This allows reducing the number of callbacks filed to
ConnectivityManager, which is a resource usage improvement and can be
beneficial for huge apps that have many components which may
collectively reach the MAX_NETWORK_REQUESTS_PER_UID limit in
ConnectivityService (100).
MdnsSocketClient is unused in AOSP so this is a no-op in the platform.
It is only used by users of the unbundled mdns library.
Bug: 322049918
Test: atest
Change-Id: I515459beaee6aacc99c7c0c2a05c3c9c41473b62
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsSocket.java b/service-t/src/com/android/server/connectivity/mdns/MdnsSocket.java
index c51811b..653ea6c 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsSocket.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsSocket.java
@@ -58,7 +58,6 @@
MdnsSocket(@NonNull MulticastNetworkInterfaceProvider multicastNetworkInterfaceProvider,
MulticastSocket multicastSocket, SharedLog sharedLog) throws IOException {
this.multicastNetworkInterfaceProvider = multicastNetworkInterfaceProvider;
- this.multicastNetworkInterfaceProvider.startWatchingConnectivityChanges();
this.multicastSocket = multicastSocket;
this.sharedLog = sharedLog;
// RFC Spec: https://tools.ietf.org/html/rfc6762
@@ -120,7 +119,6 @@
public void close() {
// This is a race with the use of the file descriptor (b/27403984).
multicastSocket.close();
- multicastNetworkInterfaceProvider.stopWatchingConnectivityChanges();
}
/**
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClient.java b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClient.java
index 82c8c5b..7b71e43 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClient.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketClient.java
@@ -106,6 +106,7 @@
@Nullable private Timer checkMulticastResponseTimer;
private final SharedLog sharedLog;
@NonNull private final MdnsFeatureFlags mdnsFeatureFlags;
+ private final MulticastNetworkInterfaceProvider interfaceProvider;
public MdnsSocketClient(@NonNull Context context, @NonNull MulticastLock multicastLock,
SharedLog sharedLog, @NonNull MdnsFeatureFlags mdnsFeatureFlags) {
@@ -118,6 +119,7 @@
unicastReceiverBuffer = null;
}
this.mdnsFeatureFlags = mdnsFeatureFlags;
+ this.interfaceProvider = new MulticastNetworkInterfaceProvider(context, sharedLog);
}
@Override
@@ -138,6 +140,7 @@
cannotReceiveMulticastResponse.set(false);
shouldStopSocketLoop = false;
+ interfaceProvider.startWatchingConnectivityChanges();
try {
// TODO (changed when importing code): consider setting thread stats tag
multicastSocket = createMdnsSocket(MdnsConstants.MDNS_PORT, sharedLog);
@@ -183,6 +186,7 @@
}
multicastLock.release();
+ interfaceProvider.stopWatchingConnectivityChanges();
shouldStopSocketLoop = true;
waitForSendThreadToStop();
@@ -482,8 +486,7 @@
@VisibleForTesting
MdnsSocket createMdnsSocket(int port, SharedLog sharedLog) throws IOException {
- return new MdnsSocket(new MulticastNetworkInterfaceProvider(context, sharedLog), port,
- sharedLog);
+ return new MdnsSocket(interfaceProvider, port, sharedLog);
}
private void sendPackets(List<DatagramPacket> packets, MdnsSocket socket) {
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketClientTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketClientTests.java
index 8b7ab71..7ced1cb 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketClientTests.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketClientTests.java
@@ -26,14 +26,17 @@
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doReturn;
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.when;
import android.Manifest.permission;
import android.annotation.RequiresPermission;
import android.content.Context;
+import android.net.ConnectivityManager;
import android.net.wifi.WifiManager;
import android.net.wifi.WifiManager.MulticastLock;
import android.text.format.DateUtils;
@@ -48,6 +51,7 @@
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -71,6 +75,7 @@
@Mock private Context mContext;
@Mock private WifiManager mockWifiManager;
+ @Mock private ConnectivityManager mockConnectivityManager;
@Mock private MdnsSocket mockMulticastSocket;
@Mock private MdnsSocket mockUnicastSocket;
@Mock private MulticastLock mockMulticastLock;
@@ -84,6 +89,9 @@
public void setup() throws RuntimeException, IOException {
MockitoAnnotations.initMocks(this);
+ doReturn(mockConnectivityManager).when(mContext).getSystemService(
+ Context.CONNECTIVITY_SERVICE);
+
when(mockWifiManager.createMulticastLock(ArgumentMatchers.anyString()))
.thenReturn(mockMulticastLock);
@@ -320,19 +328,25 @@
@Test
public void testStartStop() throws IOException {
- for (int i = 0; i < 5; i++) {
+ for (int i = 1; i <= 5; i++) {
mdnsClient.startDiscovery();
Thread multicastReceiverThread = mdnsClient.multicastReceiveThread;
Thread socketThread = mdnsClient.sendThread;
+ final ArgumentCaptor<ConnectivityManager.NetworkCallback> cbCaptor =
+ ArgumentCaptor.forClass(ConnectivityManager.NetworkCallback.class);
assertTrue(multicastReceiverThread.isAlive());
assertTrue(socketThread.isAlive());
+ verify(mockConnectivityManager, times(i))
+ .registerNetworkCallback(any(), cbCaptor.capture());
mdnsClient.stopDiscovery();
assertFalse(multicastReceiverThread.isAlive());
assertFalse(socketThread.isAlive());
+ verify(mockConnectivityManager, times(i))
+ .unregisterNetworkCallback(cbCaptor.getValue());
}
}