Fix ConnectivityManager memory leak

ConnectivityManager have a self reference static instance. This
causes a leak if context is refernce from a static variable. Using
the applicationContext, which will never be freed for the life of the
application, for the sInstance static instance.

Bug: 202978965
Test: atest ConnectivityManagerTest
Change-Id: I87206e1bfbb1f877b5a10f5fdbc25e2f9f11bef4
This commit is contained in:
markchien
2022-04-26 18:08:03 +08:00
committed by Mark Chien
parent ed0fab0356
commit d201566169
2 changed files with 19 additions and 4 deletions

View File

@@ -2589,9 +2589,24 @@ public class ConnectivityManager {
* {@hide} * {@hide}
*/ */
public ConnectivityManager(Context context, IConnectivityManager service) { public ConnectivityManager(Context context, IConnectivityManager service) {
this(context, service, true /* newStatic */);
}
private ConnectivityManager(Context context, IConnectivityManager service, boolean newStatic) {
mContext = Objects.requireNonNull(context, "missing context"); mContext = Objects.requireNonNull(context, "missing context");
mService = Objects.requireNonNull(service, "missing IConnectivityManager"); mService = Objects.requireNonNull(service, "missing IConnectivityManager");
sInstance = this; // sInstance is accessed without a lock, so it may actually be reassigned several times with
// different ConnectivityManager, but that's still OK considering its usage.
if (sInstance == null && newStatic) {
final Context appContext = mContext.getApplicationContext();
// Don't create static ConnectivityManager instance again to prevent infinite loop.
// If the application context is null, we're either in the system process or
// it's the application context very early in app initialization. In both these
// cases, the passed-in Context will not be freed, so it's safe to pass it to the
// service. http://b/27532714 .
sInstance = new ConnectivityManager(appContext != null ? appContext : context, service,
false /* newStatic */);
}
} }
/** {@hide} */ /** {@hide} */

View File

@@ -41,6 +41,7 @@ import static com.android.testutils.MiscAsserts.assertThrows;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -506,8 +507,7 @@ public class ConnectivityManagerTest {
Thread.sleep(waitIntervalMs); Thread.sleep(waitIntervalMs);
} }
// TODO: fix memory leak then assertNull here. assertNull("ConnectivityManager weak reference still not null after " + attempts
assertNotNull("Couldn't find the Context leak in ConnectivityManager after " + attempts + " attempts", ref.get());
+ " attempts", ref.get());
} }
} }