LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Address flake in testNetworkCallbackMaximum

Browse Source 
These flake occasionally because registering a request increases
the current request count synchronously while unregistering
decreases it asynchronously, meaning if the test has time to
call register 100 times before unregister can run it will
wrongfully flake.
This could be addressed in production code but as comments in
the change explain, this isn't worth the complexity. Hence
just have a pinpoint fix in the test. See aosp/2707373 for
what a fix in the production code would look like.

Test: manual
Bug: 289530922
Change-Id: Iad9a725eda91406f820abe4706bca0a4756352a4
This commit is contained in:
Chalard Jean
2023-08-29 18:00:47 +09:00
parent 29178c8906
commit 2eb2f7998d
1 changed files with 25 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
tests/unit/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -424,7 +424,6 @@ import com.android.testutils.FunctionalUtils.ThrowingConsumer;
import com.android.testutils.FunctionalUtils.ThrowingRunnable;
import com.android.testutils.HandlerUtils;
import com.android.testutils.RecorderCallback.CallbackEntry;
import com.android.testutils.SkipPresubmit;
import com.android.testutils.TestableNetworkCallback;
import com.android.testutils.TestableNetworkOfferCallback;
@@ -7430,7 +7429,6 @@ public class ConnectivityServiceTest {
assertPinnedToWifiWithCellDefault();
}
@SkipPresubmit(reason = "Out of SLO flakiness")
@Test
public void testNetworkCallbackMaximum() throws Exception {
final int MAX_REQUESTS = 100;
@@ -7549,6 +7547,19 @@ public class ConnectivityServiceTest {
NetworkCallback networkCallback = new NetworkCallback();
mCm.requestNetwork(networkRequest, networkCallback);
mCm.unregisterNetworkCallback(networkCallback);
// While requestNetwork increases the count synchronously, unregister decreases it
// asynchronously on a handler, so unregistering doesn't immediately free up
// a slot : calling unregister-register when max requests are registered throws.
// Potential fix : ConnectivityService catches TooManyRequestsException once when
// creating NetworkRequestInfo and waits for handler thread (see
// https://r.android.com/2707373 for impl). However, this complexity is not equal to
// the issue ; the purpose of having "max requests" is only to help apps detect leaks.
// Apps relying on exact enforcement or rapid request registration should reconsider.
//
// In this test, test thread registering all before handler thread decrements can cause
// flakes. A single waitForIdle at (e.g.) MAX_REQUESTS / 2 processes decrements up to
// that point, fixing the flake.
if (MAX_REQUESTS / 2 == i) waitForIdle();
}
waitForIdle();
@@ -7556,6 +7567,8 @@ public class ConnectivityServiceTest {
NetworkCallback networkCallback = new NetworkCallback();
mCm.registerNetworkCallback(networkRequest, networkCallback);
mCm.unregisterNetworkCallback(networkCallback);
// See comment above for the reasons for this wait.
if (MAX_REQUESTS / 2 == i) waitForIdle();
}
waitForIdle();
@@ -7563,6 +7576,8 @@ public class ConnectivityServiceTest {
NetworkCallback networkCallback = new NetworkCallback();
mCm.registerDefaultNetworkCallback(networkCallback);
mCm.unregisterNetworkCallback(networkCallback);
// See comment above for the reasons for this wait.
if (MAX_REQUESTS / 2 == i) waitForIdle();
}
waitForIdle();
@@ -7570,6 +7585,8 @@ public class ConnectivityServiceTest {
NetworkCallback networkCallback = new NetworkCallback();
mCm.registerDefaultNetworkCallback(networkCallback);
mCm.unregisterNetworkCallback(networkCallback);
// See comment above for the reasons for this wait.
if (MAX_REQUESTS / 2 == i) waitForIdle();
}
waitForIdle();
@@ -7579,6 +7596,8 @@ public class ConnectivityServiceTest {
mCm.registerDefaultNetworkCallbackForUid(1000000 + i, networkCallback,
new Handler(ConnectivityThread.getInstanceLooper()));
mCm.unregisterNetworkCallback(networkCallback);
// See comment above for the reasons for this wait.
if (MAX_REQUESTS / 2 == i) waitForIdle();
}
});
waitForIdle();
@@ -7588,6 +7607,8 @@ public class ConnectivityServiceTest {
mContext, 0 /* requestCode */, new Intent("e" + i), FLAG_IMMUTABLE);
mCm.requestNetwork(networkRequest, pendingIntent);
mCm.unregisterNetworkCallback(pendingIntent);
// See comment above for the reasons for this wait.
if (MAX_REQUESTS / 2 == i) waitForIdle();
}
waitForIdle();
@@ -7596,6 +7617,8 @@ public class ConnectivityServiceTest {
mContext, 0 /* requestCode */, new Intent("f" + i), FLAG_IMMUTABLE);
mCm.registerNetworkCallback(networkRequest, pendingIntent);
mCm.unregisterNetworkCallback(pendingIntent);
// See comment above for the reasons for this wait.
if (MAX_REQUESTS / 2 == i) waitForIdle();
}
}