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

Merge "Fix TetheringManager memory leak"

Browse Source
This commit is contained in:
Treehugger Robot
2021-10-20 03:59:59 +00:00
committed by Gerrit Code Review
parent 9755b43575 1110b725a0
commit d7d41a73e7
3 changed files with 79 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
Tethering/common/TetheringLib/src/android/net/TetheringManager.java
View File

@@ -37,6 +37,7 @@ import com.android.internal.annotations.GuardedBy;
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 class TetheringManager {
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 @@ public class TetheringManager {
}
}
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 @@ public class TetheringManager {
}
}
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 @@ public class TetheringManager {
@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

5
Tethering/tests/unit/src/com/android/networkstack/tethering/MockTetheringService.java
View File

@@ -22,7 +22,6 @@ import static org.mockito.Mockito.mock;
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 @@ public class MockTetheringService extends TetheringService {
mBase = base;
}
public ITetheringConnector getTetheringConnector() {
return ITetheringConnector.Stub.asInterface(mBase);
public IBinder getIBinder() {
return mBase;
}
public MockTetheringService getService() {

46
Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
View File

@@ -26,6 +26,8 @@ import static android.net.TetheringManager.TETHER_ERROR_NO_CHANGE_TETHERING_PERM
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.content.Intent;
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.Before;
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 @@ public final class TetheringServiceTest {
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 @@ public final class TetheringServiceTest {
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());
});
}
}