From d1f8cea4dfa47685353ffb4a8584ac53aea69bc0 Mon Sep 17 00:00:00 2001 From: Quang Anh Luong Date: Fri, 7 Mar 2025 18:57:22 +0900 Subject: [PATCH] Only allow one pending bluetooth request at a time Currently there may be multiple identical bluetooth requests waiting for the PAN service to connect before their listeners are notified. To simplify future code, we should only allow one pending request/listener pair at a time, and reject every subsequent request with TETHER_ERROR_SERVICE_UNAVAILABLE. This should be OK because 1) Bluetooth tethering is hardly used at all 2) Settings disallows extra startTethering calls between the first call and the bluetooth service starting 3) Apps are expected to handle TETHER_ERROR_SERVICE_UNAVAILABLE correctly. Bug: 216524590 Test: atest TetheringTest Change-Id: I8375d495575715f428e893316f7cd7e5a7ebba59 --- .../networkstack/tethering/Tethering.java | 38 ++++++++++--------- .../networkstack/tethering/TetheringTest.java | 29 +++++++++++++- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 1589509ea9..03bd52f544 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -290,7 +290,10 @@ public class Tethering { private SettingsObserver mSettingsObserver; private BluetoothPan mBluetoothPan; private PanServiceListener mBluetoothPanListener; - private final ArrayList mPendingPanRequestListeners; + // Pending listener for starting Bluetooth tethering before the PAN service is connected. Once + // the service is connected, the bluetooth iface will be requested and the listener will be + // called. + private IIntResultListener mPendingPanRequestListener; // AIDL doesn't support Set. Maintain a int bitmap here. When the bitmap is passed to // TetheringManager, TetheringManager would convert it to a set of Integer types. // mSupportedTypeBitmap should always be updated inside tethering internal thread but it may be @@ -308,11 +311,6 @@ public class Tethering { mTetheringMetrics = mDeps.makeTetheringMetrics(mContext); mRequestTracker = new RequestTracker(); - // This is intended to ensrure that if something calls startTethering(bluetooth) just after - // bluetooth is enabled. Before onServiceConnected is called, store the calls into this - // list and handle them as soon as onServiceConnected is called. - mPendingPanRequestListeners = new ArrayList<>(); - mTetherStates = new ArrayMap<>(); mConnectedClientsTracker = new ConnectedClientsTracker(); @@ -862,15 +860,21 @@ public class Tethering { if (!enable) { // The service is not connected. If disabling tethering, there's no point starting // the service just to stop tethering since tethering is not started. Just remove - // any pending requests to enable tethering, and notify them that they have failed. - for (IIntResultListener pendingListener : mPendingPanRequestListeners) { - sendTetherResult(pendingListener, TETHER_ERROR_SERVICE_UNAVAIL, + // any pending request to enable tethering, and notify them that they have failed. + if (mPendingPanRequestListener != null) { + sendTetherResult(mPendingPanRequestListener, TETHER_ERROR_SERVICE_UNAVAIL, TETHERING_BLUETOOTH); } - mPendingPanRequestListeners.clear(); + mPendingPanRequestListener = null; return TETHER_ERROR_NO_ERROR; } - mPendingPanRequestListeners.add(listener); + + // Only allow one pending request at a time. + if (mPendingPanRequestListener != null) { + return TETHER_ERROR_SERVICE_UNAVAIL; + } + + mPendingPanRequestListener = listener; // Bluetooth tethering is not a popular feature. To avoid bind to bluetooth pan service all // the time but user never use bluetooth tethering. mBluetoothPanListener is created first @@ -897,12 +901,12 @@ public class Tethering { mBluetoothPan = (BluetoothPan) proxy; mIsConnected = true; - for (IIntResultListener pendingListener : mPendingPanRequestListeners) { + if (mPendingPanRequestListener != null) { final int result = setBluetoothTetheringSettings(mBluetoothPan, true /* enable */); - sendTetherResult(pendingListener, result, TETHERING_BLUETOOTH); + sendTetherResult(mPendingPanRequestListener, result, TETHERING_BLUETOOTH); } - mPendingPanRequestListeners.clear(); + mPendingPanRequestListener = null; }); } @@ -913,11 +917,11 @@ public class Tethering { // reachable before next onServiceConnected. mIsConnected = false; - for (IIntResultListener pendingListener : mPendingPanRequestListeners) { - sendTetherResult(pendingListener, TETHER_ERROR_SERVICE_UNAVAIL, + if (mPendingPanRequestListener != null) { + sendTetherResult(mPendingPanRequestListener, TETHER_ERROR_SERVICE_UNAVAIL, TETHERING_BLUETOOTH); } - mPendingPanRequestListeners.clear(); + mPendingPanRequestListener = null; mBluetoothIfaceRequest = null; mBluetoothCallback = null; maybeDisableBluetoothIpServing(); 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 51efaf8746..2a22c6d998 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -101,6 +101,7 @@ import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; @@ -3571,6 +3572,32 @@ public class TetheringTest { failedEnable.assertHasResult(); } + @Test + public void testStartBluetoothTetheringFailsWhenTheresAnExistingRequestWaitingForPanService() + throws Exception { + initTetheringOnTestThread(); + + mockBluetoothSettings(true /* bluetoothOn */, true /* tetheringOn */); + final ResultListener firstResult = new ResultListener(TETHER_ERROR_NO_ERROR); + mTethering.startTethering(createTetheringRequest(TETHERING_BLUETOOTH), + TEST_CALLER_PKG, firstResult); + mLooper.dispatchAll(); + firstResult.assertDoesNotHaveResult(); + + // Second request should fail. + final ResultListener secondResult = new ResultListener(TETHER_ERROR_SERVICE_UNAVAIL); + mTethering.startTethering(createTetheringRequest(TETHERING_BLUETOOTH), + TEST_CALLER_PKG, secondResult); + mLooper.dispatchAll(); + secondResult.assertHasResult(); + firstResult.assertDoesNotHaveResult(); + + // Bind to PAN service should succeed for first listener only. If the second result is + // called with TETHER_ERROR_NO_ERROR, ResultListener will fail an assertion. + verifySetBluetoothTethering(true /* enable */, true /* bindToPanService */); + firstResult.assertHasResult(); + } + private void mockBluetoothSettings(boolean bluetoothOn, boolean tetheringOn) { when(mBluetoothAdapter.isEnabled()).thenReturn(bluetoothOn); when(mBluetoothPan.isTetheringOn()).thenReturn(tetheringOn); @@ -3618,7 +3645,7 @@ public class TetheringTest { private ServiceListener verifySetBluetoothTethering(final boolean enable, final boolean bindToPanService) throws Exception { ServiceListener listener = null; - verify(mBluetoothAdapter).isEnabled(); + verify(mBluetoothAdapter, atLeastOnce()).isEnabled(); if (bindToPanService) { final ArgumentCaptor listenerCaptor = ArgumentCaptor.forClass(ServiceListener.class);