Fix crash and duplicated ethernet tethering request
This change fix two things:
1. Handle ethernet callback in internal thread to avoid crash. IpServer
should be created from tethering thread, otherwise mIpNeighborMonitor of
IpServer would throw
IllegalStateException("start() called from off-thread")
2. Ethernet tethering request may be duplicated if multiple
startTethering is called but no stopTethering
Bug: 130840861
Bug: 148824036
Test: ON/OFF ethernet tehtering manually
atest TetheringTests
Change-Id: Ibd3ea6bc6751bd65647ff381f9b0124bc3395c09
Merged-In: I7c5127e96d80d077735010d2e62c7227805ccb10
Merged-In: Ibd3ea6bc6751bd65647ff381f9b0124bc3395c09
(cherry picked from commit 72702b979654234c18045f04270040056a74cf90)
diff --git a/api/system-current.txt b/api/system-current.txt
index 0c44600..32a3b6f 100755
--- a/api/system-current.txt
+++ b/api/system-current.txt
@@ -6128,7 +6128,7 @@
}
public class EthernetManager {
- method @NonNull public android.net.EthernetManager.TetheredInterfaceRequest requestTetheredInterface(@NonNull android.net.EthernetManager.TetheredInterfaceCallback);
+ method @NonNull public android.net.EthernetManager.TetheredInterfaceRequest requestTetheredInterface(@NonNull java.util.concurrent.Executor, @NonNull android.net.EthernetManager.TetheredInterfaceCallback);
}
public static interface EthernetManager.TetheredInterfaceCallback {
diff --git a/api/test-current.txt b/api/test-current.txt
index d03539c..d0958ef 100644
--- a/api/test-current.txt
+++ b/api/test-current.txt
@@ -1727,7 +1727,7 @@
}
public class EthernetManager {
- method @NonNull public android.net.EthernetManager.TetheredInterfaceRequest requestTetheredInterface(@NonNull android.net.EthernetManager.TetheredInterfaceCallback);
+ method @NonNull public android.net.EthernetManager.TetheredInterfaceRequest requestTetheredInterface(@NonNull java.util.concurrent.Executor, @NonNull android.net.EthernetManager.TetheredInterfaceCallback);
}
public static interface EthernetManager.TetheredInterfaceCallback {
diff --git a/core/java/android/net/EthernetManager.java b/core/java/android/net/EthernetManager.java
index a3899b7..139f5be 100644
--- a/core/java/android/net/EthernetManager.java
+++ b/core/java/android/net/EthernetManager.java
@@ -28,6 +28,7 @@
import java.util.ArrayList;
import java.util.Objects;
+import java.util.concurrent.Executor;
/**
* A class representing the IP configuration of the Ethernet network.
@@ -248,18 +249,19 @@
* @param callback A callback to be called once the request has been fulfilled.
*/
@NonNull
- public TetheredInterfaceRequest requestTetheredInterface(
- @NonNull TetheredInterfaceCallback callback) {
+ public TetheredInterfaceRequest requestTetheredInterface(@NonNull final Executor executor,
+ @NonNull final TetheredInterfaceCallback callback) {
Objects.requireNonNull(callback, "Callback must be non-null");
+ Objects.requireNonNull(executor, "Executor must be non-null");
final ITetheredInterfaceCallback cbInternal = new ITetheredInterfaceCallback.Stub() {
@Override
public void onAvailable(String iface) {
- callback.onAvailable(iface);
+ executor.execute(() -> callback.onAvailable(iface));
}
@Override
public void onUnavailable() {
- callback.onUnavailable();
+ executor.execute(() -> callback.onUnavailable());
}
};
diff --git a/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java b/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java
index ca74430..864f35c 100644
--- a/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java
+++ b/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java
@@ -220,6 +220,7 @@
private final UserRestrictionActionListener mTetheringRestriction;
private final ActiveDataSubIdListener mActiveDataSubIdListener;
private final ConnectedClientsTracker mConnectedClientsTracker;
+ private final TetheringThreadExecutor mExecutor;
private int mActiveDataSubId = INVALID_SUBSCRIPTION_ID;
// All the usage of mTetheringEventCallback should run in the same thread.
private ITetheringEventCallback mTetheringEventCallback = null;
@@ -296,8 +297,8 @@
final UserManager userManager = (UserManager) mContext.getSystemService(
Context.USER_SERVICE);
mTetheringRestriction = new UserRestrictionActionListener(userManager, this);
- final TetheringThreadExecutor executor = new TetheringThreadExecutor(mHandler);
- mActiveDataSubIdListener = new ActiveDataSubIdListener(executor);
+ mExecutor = new TetheringThreadExecutor(mHandler);
+ mActiveDataSubIdListener = new ActiveDataSubIdListener(mExecutor);
// Load tethering configuration.
updateConfiguration();
@@ -315,9 +316,7 @@
final WifiManager wifiManager = getWifiManager();
if (wifiManager != null) {
- wifiManager.registerSoftApCallback(
- mHandler::post /* executor */,
- new TetheringSoftApCallback());
+ wifiManager.registerSoftApCallback(mExecutor, new TetheringSoftApCallback());
}
}
@@ -606,14 +605,17 @@
Context.ETHERNET_SERVICE);
synchronized (mPublicSync) {
if (enable) {
+ if (mEthernetCallback != null) return TETHER_ERROR_NO_ERROR;
+
mEthernetCallback = new EthernetCallback();
- mEthernetIfaceRequest = em.requestTetheredInterface(mEthernetCallback);
+ mEthernetIfaceRequest = em.requestTetheredInterface(mExecutor, mEthernetCallback);
} else {
- if (mConfiguredEthernetIface != null) {
- stopEthernetTetheringLocked();
+ stopEthernetTetheringLocked();
+ if (mEthernetCallback != null) {
mEthernetIfaceRequest.release();
+ mEthernetCallback = null;
+ mEthernetIfaceRequest = null;
}
- mEthernetCallback = null;
}
}
return TETHER_ERROR_NO_ERROR;
diff --git a/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java b/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java
index f2074bd..72c3435 100644
--- a/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java
+++ b/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java
@@ -28,6 +28,7 @@
import static android.net.TetheringManager.EXTRA_ACTIVE_LOCAL_ONLY;
import static android.net.TetheringManager.EXTRA_ACTIVE_TETHER;
import static android.net.TetheringManager.EXTRA_AVAILABLE_TETHER;
+import static android.net.TetheringManager.TETHERING_ETHERNET;
import static android.net.TetheringManager.TETHERING_NCM;
import static android.net.TetheringManager.TETHERING_USB;
import static android.net.TetheringManager.TETHERING_WIFI;
@@ -75,6 +76,8 @@
import android.content.res.Resources;
import android.hardware.usb.UsbManager;
import android.net.ConnectivityManager;
+import android.net.EthernetManager;
+import android.net.EthernetManager.TetheredInterfaceRequest;
import android.net.INetd;
import android.net.ITetheringEventCallback;
import android.net.InetAddresses;
@@ -180,6 +183,7 @@
@Mock private UserManager mUserManager;
@Mock private NetworkRequest mNetworkRequest;
@Mock private ConnectivityManager mCm;
+ @Mock private EthernetManager mEm;
private final MockIpServerDependencies mIpServerDependencies =
spy(new MockIpServerDependencies());
@@ -232,6 +236,7 @@
if (Context.USER_SERVICE.equals(name)) return mUserManager;
if (Context.NETWORK_STATS_SERVICE.equals(name)) return mStatsManager;
if (Context.CONNECTIVITY_SERVICE.equals(name)) return mCm;
+ if (Context.ETHERNET_SERVICE.equals(name)) return mEm;
return super.getSystemService(name);
}
@@ -1316,6 +1321,24 @@
assertEquals(fakeSubId, newConfig.activeDataSubId);
}
+ @Test
+ public void testNoDuplicatedEthernetRequest() throws Exception {
+ final TetheredInterfaceRequest mockRequest = mock(TetheredInterfaceRequest.class);
+ when(mEm.requestTetheredInterface(any(), any())).thenReturn(mockRequest);
+ mTethering.startTethering(createTetheringRquestParcel(TETHERING_ETHERNET), null);
+ mLooper.dispatchAll();
+ verify(mEm, times(1)).requestTetheredInterface(any(), any());
+ mTethering.startTethering(createTetheringRquestParcel(TETHERING_ETHERNET), null);
+ mLooper.dispatchAll();
+ verifyNoMoreInteractions(mEm);
+ mTethering.stopTethering(TETHERING_ETHERNET);
+ mLooper.dispatchAll();
+ verify(mockRequest, times(1)).release();
+ mTethering.stopTethering(TETHERING_ETHERNET);
+ mLooper.dispatchAll();
+ verifyNoMoreInteractions(mEm);
+ }
+
private void workingWifiP2pGroupOwner(
boolean emulateInterfaceStatusChanged) throws Exception {
if (emulateInterfaceStatusChanged) {