From 13921b2cdc6df2d7456754bf4e1ce79ed305fcf1 Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Mon, 20 Mar 2023 15:06:05 +0800 Subject: [PATCH] Clean up the LinkProperties when stop monitoring sockets The active network LinkProperties are not cleaned up after stop socket monitoring. Then the apps will have duplicate sockets to discover on one interface if the interface has been toggled, because requestSocket will return both the old socket (for the old netId that is actually gone) and a new socket on the same interface, for the new netId. Thus, clean up the LinkProperties when stop monitoring sockets. Bug: 274063187 Test: atest FrameworksNetTests android.net.cts.NsdManagerTest Change-Id: I3d600169b069a1534bd740e711bc21db107b1201 --- .../connectivity/mdns/MdnsSocketProvider.java | 6 +++ .../mdns/MdnsSocketProviderTest.java | 54 ++++++++++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java index 743f946558..1d9a24c3e3 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsSocketProvider.java @@ -211,6 +211,12 @@ public class MdnsSocketProvider { tetheringManager.unregisterTetheringEventCallback(mTetheringEventCallback); mHandler.post(mNetlinkMonitor::stop); + // Clear all saved status. + mActiveNetworksLinkProperties.clear(); + mNetworkSockets.clear(); + mTetherInterfaceSockets.clear(); + mLocalOnlyInterfaces.clear(); + mTetheredInterfaces.clear(); mMonitoringSockets = false; } } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java index b9cb255e03..6e1debe292 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsSocketProviderTest.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.any; import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.doReturn; @@ -101,7 +102,7 @@ public class MdnsSocketProviderTest { doCallRealMethod().when(mContext).getSystemService(TetheringManager.class); } doReturn(true).when(mDeps).canScanOnInterface(any()); - doReturn(mTestNetworkIfaceWrapper).when(mDeps).getNetworkInterfaceByName(TEST_IFACE_NAME); + doReturn(mTestNetworkIfaceWrapper).when(mDeps).getNetworkInterfaceByName(anyString()); doReturn(mLocalOnlyIfaceWrapper).when(mDeps) .getNetworkInterfaceByName(LOCAL_ONLY_IFACE_NAME); doReturn(mTetheredIfaceWrapper).when(mDeps).getNetworkInterfaceByName(TETHERED_IFACE_NAME); @@ -354,4 +355,55 @@ public class MdnsSocketProviderTest { verify(mCm, times(2)).unregisterNetworkCallback(any(NetworkCallback.class)); verify(mTm, times(2)).unregisterTetheringEventCallback(any(TetheringEventCallback.class)); } + + @Test + public void testLinkPropertiesAreClearedAfterStopMonitoringSockets() { + startMonitoringSockets(); + + // Request a socket with null network. + final TestSocketCallback testCallback = new TestSocketCallback(); + mHandler.post(() -> mSocketProvider.requestSocket(null, testCallback)); + HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); + testCallback.expectedNoCallback(); + + // Notify a LinkPropertiesChanged with TEST_NETWORK. + final LinkProperties testLp = new LinkProperties(); + testLp.setInterfaceName(TEST_IFACE_NAME); + testLp.setLinkAddresses(List.of(LINKADDRV4)); + mHandler.post(() -> mNetworkCallback.onLinkPropertiesChanged(TEST_NETWORK, testLp)); + HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); + verify(mTestNetworkIfaceWrapper, times(1)).getNetworkInterface(); + testCallback.expectedSocketCreatedForNetwork(TEST_NETWORK, List.of(LINKADDRV4)); + + // Try to stop monitoring and unrequest the socket. + mHandler.post(mSocketProvider::requestStopWhenInactive); + HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); + mHandler.post(()-> mSocketProvider.unrequestSocket(testCallback)); + HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); + testCallback.expectedInterfaceDestroyedForNetwork(TEST_NETWORK); + verify(mCm, times(1)).unregisterNetworkCallback(any(NetworkCallback.class)); + verify(mTm, times(1)).unregisterTetheringEventCallback(any()); + + // Start sockets monitoring and request a socket again. Expected no socket created callback + // because all saved LinkProperties has been cleared. + mHandler.post(mSocketProvider::startMonitoringSockets); + HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); + verify(mCm, times(2)).registerNetworkCallback(any(), any(NetworkCallback.class), any()); + verify(mTm, times(2)).registerTetheringEventCallback( + any(), any(TetheringEventCallback.class)); + mHandler.post(() -> mSocketProvider.requestSocket(null, testCallback)); + HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); + testCallback.expectedNoCallback(); + + // Notify a LinkPropertiesChanged with another network. + final LinkProperties otherLp = new LinkProperties(); + final LinkAddress otherAddress = new LinkAddress("192.0.2.1/24"); + final Network otherNetwork = new Network(456); + otherLp.setInterfaceName("test2"); + otherLp.setLinkAddresses(List.of(otherAddress)); + mHandler.post(() -> mNetworkCallback.onLinkPropertiesChanged(otherNetwork, otherLp)); + HandlerUtils.waitForIdle(mHandler, DEFAULT_TIMEOUT); + verify(mTestNetworkIfaceWrapper, times(2)).getNetworkInterface(); + testCallback.expectedSocketCreatedForNetwork(otherNetwork, List.of(otherAddress)); + } }