Put tether/untether calls into handler queue
BT tethering is the only user of #tether and #untether API.
Enable BT tethering may fail by race between interface status change
and #tether call. In starting BT tethering flow, tethering may receive
pan interface DOWN and UP then PanService call #tether to request for
processing its pan interface. Tethering put interface status change
event into handler queue but it process #tether and #unether calls
right away without order them by queue. It makes tethering handle
binder call #tether before pan interface DOWN even DOWN event arrived
eailer. This cause BT tethering bring down by interface DOWN event
unexpectedly. Although this still can't fix all the race cases of
tethering, but at lesat this could fix the race mentioned above.
Bug: 173310882
Test: manual ON/OFF bt tethering
atest TetheringTests
Change-Id: I2411378aa36ad4371cca12423bb65542cb8df7a1
diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 62ae88c..0cf6fcd 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -747,8 +747,12 @@
}
}
- int tether(String iface) {
- return tether(iface, IpServer.STATE_TETHERED);
+ void tether(String iface, final IIntResultListener listener) {
+ mHandler.post(() -> {
+ try {
+ listener.onResult(tether(iface, IpServer.STATE_TETHERED));
+ } catch (RemoteException e) { }
+ });
}
private int tether(String iface, int requestedState) {
@@ -781,6 +785,14 @@
}
}
+ void untether(String iface, final IIntResultListener listener) {
+ mHandler.post(() -> {
+ try {
+ listener.onResult(untether(iface));
+ } catch (RemoteException e) { }
+ });
+ }
+
int untether(String iface) {
if (DBG) Log.d(TAG, "Untethering " + iface);
synchronized (mPublicSync) {
diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringService.java b/Tethering/src/com/android/networkstack/tethering/TetheringService.java
index 613328d..400b31e 100644
--- a/Tethering/src/com/android/networkstack/tethering/TetheringService.java
+++ b/Tethering/src/com/android/networkstack/tethering/TetheringService.java
@@ -106,9 +106,7 @@
IIntResultListener listener) {
if (checkAndNotifyCommonError(callerPkg, callingAttributionTag, listener)) return;
- try {
- listener.onResult(mTethering.tether(iface));
- } catch (RemoteException e) { }
+ mTethering.tether(iface, listener);
}
@Override
@@ -116,9 +114,7 @@
IIntResultListener listener) {
if (checkAndNotifyCommonError(callerPkg, callingAttributionTag, listener)) return;
- try {
- listener.onResult(mTethering.untether(iface));
- } catch (RemoteException e) { }
+ mTethering.untether(iface, listener);
}
@Override
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 7bba67b..be98f60 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
@@ -156,11 +156,9 @@
}
private void runTether(final TestTetheringResult result) throws Exception {
- when(mTethering.tether(TEST_IFACE_NAME)).thenReturn(TETHER_ERROR_NO_ERROR);
mTetheringConnector.tether(TEST_IFACE_NAME, TEST_CALLER_PKG, TEST_ATTRIBUTION_TAG, result);
verify(mTethering).isTetheringSupported();
- verify(mTethering).tether(TEST_IFACE_NAME);
- result.assertResult(TETHER_ERROR_NO_ERROR);
+ verify(mTethering).tether(eq(TEST_IFACE_NAME), eq(result));
}
@Test
@@ -186,12 +184,10 @@
}
private void runUnTether(final TestTetheringResult result) throws Exception {
- when(mTethering.untether(TEST_IFACE_NAME)).thenReturn(TETHER_ERROR_NO_ERROR);
mTetheringConnector.untether(TEST_IFACE_NAME, TEST_CALLER_PKG, TEST_ATTRIBUTION_TAG,
result);
verify(mTethering).isTetheringSupported();
- verify(mTethering).untether(TEST_IFACE_NAME);
- result.assertResult(TETHER_ERROR_NO_ERROR);
+ verify(mTethering).untether(eq(TEST_IFACE_NAME), eq(result));
}
@Test
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index f4b3749..53c35f8 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -33,6 +33,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_BLUETOOTH;
import static android.net.TetheringManager.TETHERING_ETHERNET;
import static android.net.TetheringManager.TETHERING_NCM;
import static android.net.TetheringManager.TETHERING_USB;
@@ -85,6 +86,9 @@
import android.app.usage.NetworkStatsManager;
import android.bluetooth.BluetoothAdapter;
+import android.bluetooth.BluetoothPan;
+import android.bluetooth.BluetoothProfile;
+import android.bluetooth.BluetoothProfile.ServiceListener;
import android.content.BroadcastReceiver;
import android.content.ContentResolver;
import android.content.Context;
@@ -226,6 +230,8 @@
@Mock private TetheringNotificationUpdater mNotificationUpdater;
@Mock private BpfCoordinator mBpfCoordinator;
@Mock private PackageManager mPackageManager;
+ @Mock private BluetoothAdapter mBluetoothAdapter;
+ @Mock private BluetoothPan mBluetoothPan;
private final MockIpServerDependencies mIpServerDependencies =
spy(new MockIpServerDependencies());
@@ -320,7 +326,8 @@
|| ifName.equals(TEST_MOBILE_IFNAME)
|| ifName.equals(TEST_P2P_IFNAME)
|| ifName.equals(TEST_NCM_IFNAME)
- || ifName.equals(TEST_ETH_IFNAME));
+ || ifName.equals(TEST_ETH_IFNAME)
+ || ifName.equals(TEST_BT_IFNAME));
final String[] ifaces = new String[] {
TEST_USB_IFNAME, TEST_WLAN_IFNAME, TEST_MOBILE_IFNAME, TEST_P2P_IFNAME,
TEST_NCM_IFNAME, TEST_ETH_IFNAME};
@@ -452,8 +459,7 @@
@Override
public BluetoothAdapter getBluetoothAdapter() {
- // TODO: add test for bluetooth tethering.
- return null;
+ return mBluetoothAdapter;
}
@Override
@@ -550,7 +556,7 @@
when(mNetd.interfaceGetList())
.thenReturn(new String[] {
TEST_MOBILE_IFNAME, TEST_WLAN_IFNAME, TEST_USB_IFNAME, TEST_P2P_IFNAME,
- TEST_NCM_IFNAME, TEST_ETH_IFNAME});
+ TEST_NCM_IFNAME, TEST_ETH_IFNAME, TEST_BT_IFNAME});
when(mResources.getString(R.string.config_wifi_tether_enable)).thenReturn("");
mInterfaceConfiguration = new InterfaceConfigurationParcel();
mInterfaceConfiguration.flags = new String[0];
@@ -597,11 +603,11 @@
when(mResources.getStringArray(R.array.config_tether_usb_regexs))
.thenReturn(new String[] { "test_rndis\\d" });
when(mResources.getStringArray(R.array.config_tether_wifi_regexs))
- .thenReturn(new String[]{ "test_wlan\\d" });
+ .thenReturn(new String[] { "test_wlan\\d" });
when(mResources.getStringArray(R.array.config_tether_wifi_p2p_regexs))
- .thenReturn(new String[]{ "test_p2p-p2p\\d-.*" });
+ .thenReturn(new String[] { "test_p2p-p2p\\d-.*" });
when(mResources.getStringArray(R.array.config_tether_bluetooth_regexs))
- .thenReturn(new String[0]);
+ .thenReturn(new String[] { "test_pan\\d" });
when(mResources.getStringArray(R.array.config_tether_ncm_regexs))
.thenReturn(new String[] { "test_ncm\\d" });
when(mResources.getIntArray(R.array.config_tether_upstream_types)).thenReturn(new int[0]);
@@ -2190,6 +2196,70 @@
return lease;
}
+ @Test
+ public void testBluetoothTethering() throws Exception {
+ final ResultListener result = new ResultListener(TETHER_ERROR_NO_ERROR);
+ when(mBluetoothAdapter.isEnabled()).thenReturn(true);
+ mTethering.startTethering(createTetheringRequestParcel(TETHERING_BLUETOOTH), result);
+ mLooper.dispatchAll();
+ verifySetBluetoothTethering(true);
+ result.assertHasResult();
+
+ mTethering.interfaceAdded(TEST_BT_IFNAME);
+ mLooper.dispatchAll();
+
+ mTethering.interfaceStatusChanged(TEST_BT_IFNAME, false);
+ mTethering.interfaceStatusChanged(TEST_BT_IFNAME, true);
+ final ResultListener tetherResult = new ResultListener(TETHER_ERROR_NO_ERROR);
+ mTethering.tether(TEST_BT_IFNAME, tetherResult);
+ mLooper.dispatchAll();
+ tetherResult.assertHasResult();
+
+ verify(mNetd).tetherInterfaceAdd(TEST_BT_IFNAME);
+ verify(mNetd).networkAddInterface(INetd.LOCAL_NET_ID, TEST_BT_IFNAME);
+ verify(mNetd, times(2)).networkAddRoute(eq(INetd.LOCAL_NET_ID), eq(TEST_BT_IFNAME),
+ anyString(), anyString());
+ verify(mNetd).ipfwdEnableForwarding(TETHERING_NAME);
+ verify(mNetd).tetherStartWithConfiguration(any());
+ verify(mNetd, times(2)).networkAddRoute(eq(INetd.LOCAL_NET_ID), eq(TEST_BT_IFNAME),
+ anyString(), anyString());
+ verifyNoMoreInteractions(mNetd);
+ reset(mNetd);
+
+ when(mBluetoothAdapter.isEnabled()).thenReturn(true);
+ mTethering.stopTethering(TETHERING_BLUETOOTH);
+ mLooper.dispatchAll();
+ final ResultListener untetherResult = new ResultListener(TETHER_ERROR_NO_ERROR);
+ mTethering.untether(TEST_BT_IFNAME, untetherResult);
+ mLooper.dispatchAll();
+ untetherResult.assertHasResult();
+ verifySetBluetoothTethering(false);
+
+ verify(mNetd).tetherApplyDnsInterfaces();
+ verify(mNetd).tetherInterfaceRemove(TEST_BT_IFNAME);
+ verify(mNetd).networkRemoveInterface(INetd.LOCAL_NET_ID, TEST_BT_IFNAME);
+ verify(mNetd).interfaceSetCfg(any(InterfaceConfigurationParcel.class));
+ verify(mNetd).tetherStop();
+ verify(mNetd).ipfwdDisableForwarding(TETHERING_NAME);
+ verifyNoMoreInteractions(mNetd);
+ }
+
+ private void verifySetBluetoothTethering(final boolean enable) {
+ final ArgumentCaptor<ServiceListener> listenerCaptor =
+ ArgumentCaptor.forClass(ServiceListener.class);
+ verify(mBluetoothAdapter).isEnabled();
+ verify(mBluetoothAdapter).getProfileProxy(eq(mServiceContext), listenerCaptor.capture(),
+ eq(BluetoothProfile.PAN));
+ final ServiceListener listener = listenerCaptor.getValue();
+ when(mBluetoothPan.isTetheringOn()).thenReturn(enable);
+ listener.onServiceConnected(BluetoothProfile.PAN, mBluetoothPan);
+ verify(mBluetoothPan).setBluetoothTethering(enable);
+ verify(mBluetoothPan).isTetheringOn();
+ verify(mBluetoothAdapter).closeProfileProxy(eq(BluetoothProfile.PAN), eq(mBluetoothPan));
+ verifyNoMoreInteractions(mBluetoothAdapter, mBluetoothPan);
+ reset(mBluetoothAdapter, mBluetoothPan);
+ }
+
// TODO: Test that a request for hotspot mode doesn't interfere with an
// already operating tethering mode interface.
}