From 0a9dbc9d12d76590ee94bb7c426123d6bc03e7c7 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 13 Mar 2024 19:00:58 +0900 Subject: [PATCH] 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 --- .../server/connectivity/mdns/MdnsSocket.java | 2 -- .../connectivity/mdns/MdnsSocketClient.java | 7 +++++-- .../connectivity/mdns/MdnsSocketClientTests.java | 16 +++++++++++++++- 3 files changed, 20 insertions(+), 5 deletions(-) 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 c51811b310..653ea6ccad 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 @@ public class MdnsSocket { 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 class MdnsSocket { 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 82c8c5bf27..7b71e438d4 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 @@ public class MdnsSocketClient implements MdnsSocketClientBase { @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 @@ public class MdnsSocketClient implements MdnsSocketClientBase { unicastReceiverBuffer = null; } this.mdnsFeatureFlags = mdnsFeatureFlags; + this.interfaceProvider = new MulticastNetworkInterfaceProvider(context, sharedLog); } @Override @@ -138,6 +140,7 @@ public class MdnsSocketClient implements MdnsSocketClientBase { 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 @@ public class MdnsSocketClient implements MdnsSocketClientBase { } multicastLock.release(); + interfaceProvider.stopWatchingConnectivityChanges(); shouldStopSocketLoop = true; waitForSendThreadToStop(); @@ -482,8 +486,7 @@ public class MdnsSocketClient implements MdnsSocketClientBase { @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 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 8b7ab71dba..7ced1cb6fe 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.anyInt; 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.Before; 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 @@ public class MdnsSocketClientTests { @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 class MdnsSocketClientTests { 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 @@ public class MdnsSocketClientTests { @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 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()); } }