joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge "Fix leak of NetworkAgentWrapper HandlerThread" into main

Browse source
This commit is contained in:
Remi NGUYEN VAN 2024-01-11 10:33:21 +00:00 • committed by Gerrit Code Review
commit 64a8ead5d1
3 changed files with 78 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

55
tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
View file

@ -56,9 +56,11 @@ import com.android.server.connectivity.ConnectivityResources
import com.android.server.connectivity.MockableSystemProperties
import com.android.server.connectivity.MultinetworkPolicyTracker
import com.android.server.connectivity.ProxyTracker
import com.android.testutils.DevSdkIgnoreRunner
import com.android.testutils.DeviceInfoUtils
import com.android.testutils.RecorderCallback.CallbackEntry.LinkPropertiesChanged
import com.android.testutils.TestableNetworkCallback
import com.android.testutils.tryTest
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
@ -254,13 +256,18 @@ class ConnectivityServiceIntegrationTest {
na.addCapability(NET_CAPABILITY_INTERNET)
na.connect()
testCallback.expectAvailableThenValidatedCallbacks(na.network, TEST_TIMEOUT_MS)
val requestedSize = nsInstrumentation.getRequestUrls().size
if (requestedSize == 2 || (requestedSize == 1 &&
nsInstrumentation.getRequestUrls()[0] == httpsProbeUrl)) {
return
tryTest {
testCallback.expectAvailableThenValidatedCallbacks(na.network, TEST_TIMEOUT_MS)
val requestedSize = nsInstrumentation.getRequestUrls().size
if (requestedSize == 2 || (requestedSize == 1 &&
nsInstrumentation.getRequestUrls()[0] == httpsProbeUrl)
) {
return@tryTest
}
fail("Unexpected request urls: ${nsInstrumentation.getRequestUrls()}")
} cleanup {
na.destroy()
}
fail("Unexpected request urls: ${nsInstrumentation.getRequestUrls()}")
}
@Test
@ -292,24 +299,32 @@ class ConnectivityServiceIntegrationTest {
val lp = LinkProperties()
lp.captivePortalApiUrl = Uri.parse(apiUrl)
val na = NetworkAgentWrapper(TRANSPORT_CELLULAR, lp, null /* ncTemplate */, context)
networkStackClient.verifyNetworkMonitorCreated(na.network, TEST_TIMEOUT_MS)
na.addCapability(NET_CAPABILITY_INTERNET)
na.connect()
tryTest {
networkStackClient.verifyNetworkMonitorCreated(na.network, TEST_TIMEOUT_MS)
testCb.expectAvailableCallbacks(na.network, validated = false, tmt = TEST_TIMEOUT_MS)
na.addCapability(NET_CAPABILITY_INTERNET)
na.connect()
val capportData = testCb.expect<LinkPropertiesChanged>(na, TEST_TIMEOUT_MS) {
it.lp.captivePortalData != null
}.lp.captivePortalData
assertNotNull(capportData)
assertTrue(capportData.isCaptive)
assertEquals(Uri.parse("https://login.capport.android.com"), capportData.userPortalUrl)
assertEquals(Uri.parse("https://venueinfo.capport.android.com"), capportData.venueInfoUrl)
testCb.expectAvailableCallbacks(na.network, validated = false, tmt = TEST_TIMEOUT_MS)
testCb.expectCaps(na, TEST_TIMEOUT_MS) {
it.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL) &&
!it.hasCapability(NET_CAPABILITY_VALIDATED)
val capportData = testCb.expect<LinkPropertiesChanged>(na, TEST_TIMEOUT_MS) {
it.lp.captivePortalData != null
}.lp.captivePortalData
assertNotNull(capportData)
assertTrue(capportData.isCaptive)
assertEquals(Uri.parse("https://login.capport.android.com"), capportData.userPortalUrl)
assertEquals(
Uri.parse("https://venueinfo.capport.android.com"),
capportData.venueInfoUrl
)
testCb.expectCaps(na, TEST_TIMEOUT_MS) {
it.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL) &&
!it.hasCapability(NET_CAPABILITY_VALIDATED)
}
} cleanup {
na.destroy()
}
}

36
tests/integration/util/com/android/server/NetworkAgentWrapper.java
View file

@ -36,6 +36,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import android.annotation.NonNull;
import android.annotation.SuppressLint;
import android.content.Context;
import android.net.ConnectivityManager;
import android.net.LinkProperties;
@ -51,6 +52,7 @@ import android.net.SocketKeepalive;
import android.os.ConditionVariable;
import android.os.HandlerThread;
import android.os.Message;
import android.util.CloseGuard;
import android.util.Log;
import android.util.Range;
@ -65,11 +67,14 @@ import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
private static final long DESTROY_TIMEOUT_MS = 10_000L;
// Note : Please do not add any new instrumentation here. If you need new instrumentation,
// please add it in CSAgentWrapper and use subclasses of CSTest instead of adding more
// tools in ConnectivityServiceTest.
private final NetworkCapabilities mNetworkCapabilities;
private final HandlerThread mHandlerThread;
private final CloseGuard mCloseGuard;
private final Context mContext;
private final String mLogTag;
private final NetworkAgentConfig mNetworkAgentConfig;
@ -157,6 +162,8 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
mLogTag = "Mock-" + typeName;
mHandlerThread = new HandlerThread(mLogTag);
mHandlerThread.start();
mCloseGuard = new CloseGuard();
mCloseGuard.open("destroy");
// extraInfo is set to "" by default in NetworkAgentConfig.
final String extraInfo = (transport == TRANSPORT_CELLULAR) ? "internet.apn" : "";
@ -359,6 +366,35 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
mNetworkAgent.unregister();
}
/**
* Destroy the network agent and stop its looper.
*
* <p>This must always be called.
*/
public void destroy() {
mHandlerThread.quitSafely();
try {
mHandlerThread.join(DESTROY_TIMEOUT_MS);
} catch (InterruptedException e) {
Log.e(mLogTag, "Interrupted when waiting for handler thread on destroy", e);
}
mCloseGuard.close();
}
@SuppressLint("Finalize") // Follows the recommended pattern for CloseGuard
@Override
protected void finalize() throws Throwable {
try {
// Note that mCloseGuard could be null if the constructor threw.
if (mCloseGuard != null) {
mCloseGuard.warnIfOpen();
}
destroy();
} finally {
super.finalize();
}
}
@Override
public Network getNetwork() {
return mNetworkAgent.getNetwork();

7
tests/unit/java/com/android/server/ConnectivityServiceTest.java
View file

@ -589,6 +589,7 @@ public class ConnectivityServiceTest {
private TestNetworkAgentWrapper mWiFiAgent;
private TestNetworkAgentWrapper mCellAgent;
private TestNetworkAgentWrapper mEthernetAgent;
private final List<TestNetworkAgentWrapper> mCreatedAgents = new ArrayList<>();
private MockVpn mMockVpn;
private Context mContext;
private NetworkPolicyCallback mPolicyCallback;
@ -1092,6 +1093,7 @@ public class ConnectivityServiceTest {
NetworkCapabilities ncTemplate, NetworkProvider provider,
NetworkAgentWrapper.Callbacks callbacks) throws Exception {
super(transport, linkProperties, ncTemplate, provider, callbacks, mServiceContext);
mCreatedAgents.add(this);
// Waits for the NetworkAgent to be registered, which includes the creation of the
// NetworkMonitor.
@ -2404,6 +2406,11 @@ public class ConnectivityServiceTest {
FakeSettingsProvider.clearSettingsProvider();
ConnectivityResources.setResourcesContextForTest(null);
for (TestNetworkAgentWrapper agent : mCreatedAgents) {
agent.destroy();
}
mCreatedAgents.clear();
mCsHandlerThread.quitSafely();
mCsHandlerThread.join();
mAlarmManagerThread.quitSafely();