Merge "Crash bad callers earlier"
diff --git a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java
index 69bb71f..c137b49 100644
--- a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java
+++ b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java
@@ -37,6 +37,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
+import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -265,7 +266,7 @@
public TetheringManager(@NonNull final Context context,
@NonNull Supplier<IBinder> connectorSupplier) {
mContext = context;
- mCallback = new TetheringCallbackInternal();
+ mCallback = new TetheringCallbackInternal(this);
mConnectorSupplier = connectorSupplier;
final String pkgName = mContext.getOpPackageName();
@@ -415,7 +416,7 @@
}
}
- private void throwIfPermissionFailure(final int errorCode) {
+ private static void throwIfPermissionFailure(final int errorCode) {
switch (errorCode) {
case TETHER_ERROR_NO_CHANGE_TETHERING_PERMISSION:
throw new SecurityException("No android.permission.TETHER_PRIVILEGED"
@@ -426,21 +427,40 @@
}
}
- private class TetheringCallbackInternal extends ITetheringEventCallback.Stub {
+ private static class TetheringCallbackInternal extends ITetheringEventCallback.Stub {
private volatile int mError = TETHER_ERROR_NO_ERROR;
private final ConditionVariable mWaitForCallback = new ConditionVariable();
+ // This object is never garbage collected because the Tethering code running in
+ // the system server always maintains a reference to it for as long as
+ // mCallback is registered.
+ //
+ // Don't keep a strong reference to TetheringManager because otherwise
+ // TetheringManager cannot be garbage collected, and because TetheringManager
+ // stores the Context that it was created from, this will prevent the calling
+ // Activity from being garbage collected as well.
+ private final WeakReference<TetheringManager> mTetheringMgrRef;
+
+ TetheringCallbackInternal(final TetheringManager tm) {
+ mTetheringMgrRef = new WeakReference<>(tm);
+ }
@Override
public void onCallbackStarted(TetheringCallbackStartedParcel parcel) {
- mTetheringConfiguration = parcel.config;
- mTetherStatesParcel = parcel.states;
- mWaitForCallback.open();
+ TetheringManager tetheringMgr = mTetheringMgrRef.get();
+ if (tetheringMgr != null) {
+ tetheringMgr.mTetheringConfiguration = parcel.config;
+ tetheringMgr.mTetherStatesParcel = parcel.states;
+ mWaitForCallback.open();
+ }
}
@Override
public void onCallbackStopped(int errorCode) {
- mError = errorCode;
- mWaitForCallback.open();
+ TetheringManager tetheringMgr = mTetheringMgrRef.get();
+ if (tetheringMgr != null) {
+ mError = errorCode;
+ mWaitForCallback.open();
+ }
}
@Override
@@ -448,12 +468,14 @@
@Override
public void onConfigurationChanged(TetheringConfigurationParcel config) {
- mTetheringConfiguration = config;
+ TetheringManager tetheringMgr = mTetheringMgrRef.get();
+ if (tetheringMgr != null) tetheringMgr.mTetheringConfiguration = config;
}
@Override
public void onTetherStatesChanged(TetherStatesParcel states) {
- mTetherStatesParcel = states;
+ TetheringManager tetheringMgr = mTetheringMgrRef.get();
+ if (tetheringMgr != null) tetheringMgr.mTetherStatesParcel = states;
}
@Override
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java
index 4865e03..3c07580 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java
@@ -22,7 +22,6 @@
import android.content.Context;
import android.content.Intent;
-import android.net.ITetheringConnector;
import android.os.Binder;
import android.os.IBinder;
import android.util.ArrayMap;
@@ -72,8 +71,8 @@
mBase = base;
}
- public ITetheringConnector getTetheringConnector() {
- return ITetheringConnector.Stub.asInterface(mBase);
+ public IBinder getIBinder() {
+ return mBase;
}
public MockTetheringService getService() {
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
index 1b52f6e..0052267 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
@@ -26,6 +26,8 @@
import static android.net.TetheringManager.TETHER_ERROR_NO_ERROR;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
@@ -40,10 +42,12 @@
import android.net.IIntResultListener;
import android.net.ITetheringConnector;
import android.net.ITetheringEventCallback;
+import android.net.TetheringManager;
import android.net.TetheringRequestParcel;
import android.net.ip.IpServer;
import android.os.Bundle;
import android.os.Handler;
+import android.os.IBinder;
import android.os.ResultReceiver;
import androidx.test.InstrumentationRegistry;
@@ -59,9 +63,14 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.function.Supplier;
+
@RunWith(AndroidJUnit4.class)
@SmallTest
public final class TetheringServiceTest {
@@ -113,7 +122,7 @@
InstrumentationRegistry.getTargetContext(),
MockTetheringService.class);
mMockConnector = (MockTetheringConnector) mServiceTestRule.bindService(mMockServiceIntent);
- mTetheringConnector = mMockConnector.getTetheringConnector();
+ mTetheringConnector = ITetheringConnector.Stub.asInterface(mMockConnector.getIBinder());
final MockTetheringService service = mMockConnector.getService();
mTethering = service.getTethering();
}
@@ -493,4 +502,39 @@
verifyNoMoreInteractionsForTethering();
});
}
+
+ @Test
+ public void testTetheringManagerLeak() throws Exception {
+ runAsAccessNetworkState((none) -> {
+ final ArrayList<ITetheringEventCallback> callbacks = new ArrayList<>();
+ doAnswer((invocation) -> {
+ final Object[] args = invocation.getArguments();
+ callbacks.add((ITetheringEventCallback) args[0]);
+ return null;
+ }).when(mTethering).registerTetheringEventCallback(Matchers.anyObject());
+
+ final Supplier<IBinder> supplier = () -> mMockConnector.getIBinder();
+
+ TetheringManager tm = new TetheringManager(mMockConnector.getService(), supplier);
+ assertNotNull(tm);
+ assertEquals("Internal callback is not registered", 1, callbacks.size());
+
+ final WeakReference weakTm = new WeakReference(tm);
+ assertNotNull(weakTm.get());
+
+ tm = null;
+ final int attempts = 100;
+ final long waitIntervalMs = 50;
+ for (int i = 0; i < attempts; i++) {
+ System.gc();
+ System.runFinalization();
+ System.gc();
+ if (weakTm.get() == null) break;
+
+ Thread.sleep(waitIntervalMs);
+ }
+ assertNull("TetheringManager weak reference still not null after " + attempts
+ + " attempts", weakTm.get());
+ });
+ }
}
diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
index 4329a83..9606960 100644
--- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
+++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
@@ -1183,6 +1183,7 @@
@AppModeFull(reason = "Cannot get WifiManager in instant app mode")
@Test
public void testGetMultipathPreference() throws Exception {
+ assumeTrue(mPackageManager.hasSystemFeature(FEATURE_WIFI));
final ContentResolver resolver = mContext.getContentResolver();
mCtsNetUtils.ensureWifiConnected();
final String ssid = unquoteSSID(mWifiManager.getConnectionInfo().getSSID());
@@ -2828,6 +2829,19 @@
});
}
+ /**
+ * The networks used in this test are real networks and as such they can see seemingly random
+ * updates of their capabilities or link properties as conditions change, e.g. the network
+ * loses validation or IPv4 shows up. Many tests should simply treat these callbacks as
+ * spurious.
+ */
+ private void assertNoCallbackExceptCapOrLpChange(
+ @NonNull final TestableNetworkCallback cb) {
+ cb.assertNoCallbackThat(NO_CALLBACK_TIMEOUT_MS,
+ c -> !(c instanceof CallbackEntry.CapabilitiesChanged
+ || c instanceof CallbackEntry.LinkPropertiesChanged));
+ }
+
@AppModeFull(reason = "Cannot get WifiManager in instant app mode")
@Test
public void testMobileDataPreferredUids() throws Exception {
@@ -2860,8 +2874,7 @@
// CtsNetTestCases uid is not listed in MOBILE_DATA_PREFERRED_UIDS setting, so the
// per-app default network should be same as system default network.
waitForAvailable(systemDefaultCb, wifiNetwork);
- defaultTrackingCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
- entry -> wifiNetwork.equals(entry.getNetwork()));
+ waitForAvailable(defaultTrackingCb, wifiNetwork);
// Active network for CtsNetTestCases uid should be wifi now.
assertEquals(wifiNetwork, mCm.getActiveNetwork());
@@ -2871,10 +2884,10 @@
newMobileDataPreferredUids.add(uid);
ConnectivitySettingsManager.setMobileDataPreferredUids(
mContext, newMobileDataPreferredUids);
- defaultTrackingCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
- entry -> cellNetwork.equals(entry.getNetwork()));
- // System default network doesn't change.
- systemDefaultCb.assertNoCallback();
+ waitForAvailable(defaultTrackingCb, cellNetwork);
+ // No change for system default network. Expect no callback except CapabilitiesChanged
+ // or LinkPropertiesChanged which may be triggered randomly from wifi network.
+ assertNoCallbackExceptCapOrLpChange(systemDefaultCb);
// Active network for CtsNetTestCases uid should change to cell, too.
assertEquals(cellNetwork, mCm.getActiveNetwork());
@@ -2883,10 +2896,10 @@
newMobileDataPreferredUids.remove(uid);
ConnectivitySettingsManager.setMobileDataPreferredUids(
mContext, newMobileDataPreferredUids);
- defaultTrackingCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
- entry -> wifiNetwork.equals(entry.getNetwork()));
- // System default network still doesn't change.
- systemDefaultCb.assertNoCallback();
+ waitForAvailable(defaultTrackingCb, wifiNetwork);
+ // No change for system default network. Expect no callback except CapabilitiesChanged
+ // or LinkPropertiesChanged which may be triggered randomly from wifi network.
+ assertNoCallbackExceptCapOrLpChange(systemDefaultCb);
// Active network for CtsNetTestCases uid should change back to wifi.
assertEquals(wifiNetwork, mCm.getActiveNetwork());
} finally {