diff options
| author | Jeff Sharkey <jsharkey@android.com> | 2021-08-10 16:50:52 -0600 |
|---|---|---|
| committer | Jeff Sharkey <jsharkey@android.com> | 2021-08-11 12:23:01 -0600 |
| commit | f0fa7f9b4aaee876053486d988909df9dc64e990 (patch) | |
| tree | 6271ab8e408fd731e7eec7dda3d295075f27bf07 /core/java/android/bluetooth | |
| parent | 5f51b92263c1f73fb7d7c70c6706887e9d479aa3 (diff) | |
Register IBluetoothManagerCallback per-process.
As part of introducing AttributionSource across the Bluetooth stack
earlier this year, each BluetoothAdapter instance is now associated
with a specific AttributionSource, and several instances of shared
static code were made BluetoothAdapter-specific so they could be
augmented with the relevant AttributionSource.
However, processes that create many BluetoothAdapter instances can
overload the system, since a IBluetoothManagerCallback was registered
for each instance. This change mitigates this by only registering a
single IBluetoothManagerCallback for the entire process, and it
then reuses the existing sProxyServiceStateCallbacks list for
dispatching events to all active adapters within the process.
Since it's so late in the release, we keep both mService and
sService intact to minimize the size of this CL; future work should
refactor to a more robust design, such as Supplier<IBluetooth>.
Bug: 195286998, 172022978
Test: atest BluetoothInstrumentationTests
Change-Id: I012f3f65e61eaf55e40436486806e56506c928ee
Diffstat (limited to 'core/java/android/bluetooth')
| -rw-r--r-- | core/java/android/bluetooth/BluetoothAdapter.java | 184 | ||||
| -rw-r--r-- | core/java/android/bluetooth/BluetoothSocket.java | 6 |
2 files changed, 127 insertions, 63 deletions
diff --git a/core/java/android/bluetooth/BluetoothAdapter.java b/core/java/android/bluetooth/BluetoothAdapter.java index 8398be1af49a..5094498dbee5 100644 --- a/core/java/android/bluetooth/BluetoothAdapter.java +++ b/core/java/android/bluetooth/BluetoothAdapter.java @@ -64,6 +64,8 @@ import android.os.SystemProperties; import android.util.Log; import android.util.Pair; +import com.android.internal.annotations.GuardedBy; + import java.io.IOException; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -78,6 +80,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.UUID; +import java.util.WeakHashMap; import java.util.concurrent.Executor; import java.util.concurrent.TimeoutException; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -715,10 +718,21 @@ public final class BluetoothAdapter { private final IBluetoothManager mManagerService; private final AttributionSource mAttributionSource; + // Yeah, keeping both mService and sService isn't pretty, but it's too late + // in the current release for a major refactoring, so we leave them both + // intact until this can be cleaned up in a future release + @UnsupportedAppUsage + @GuardedBy("mServiceLock") private IBluetooth mService; private final ReentrantReadWriteLock mServiceLock = new ReentrantReadWriteLock(); + @GuardedBy("sServiceLock") + private static boolean sServiceRegistered; + @GuardedBy("sServiceLock") + private static IBluetooth sService; + private static final Object sServiceLock = new Object(); + private final Object mLock = new Object(); private final Map<LeScanCallback, ScanCallback> mLeScanClients; private final Map<BluetoothDevice, List<Pair<OnMetadataChangedListener, Executor>>> @@ -792,19 +806,11 @@ public final class BluetoothAdapter { * Use {@link #getDefaultAdapter} to get the BluetoothAdapter instance. */ BluetoothAdapter(IBluetoothManager managerService, AttributionSource attributionSource) { - if (managerService == null) { - throw new IllegalArgumentException("bluetooth manager service is null"); - } - try { - mServiceLock.writeLock().lock(); - mService = managerService.registerAdapter(mManagerCallback); - } catch (RemoteException e) { - Log.e(TAG, "", e); - } finally { - mServiceLock.writeLock().unlock(); - } mManagerService = Objects.requireNonNull(managerService); mAttributionSource = Objects.requireNonNull(attributionSource); + synchronized (mServiceLock.writeLock()) { + mService = getBluetoothService(mManagerCallback); + } mLeScanClients = new HashMap<LeScanCallback, ScanCallback>(); mToken = new Binder(DESCRIPTOR); } @@ -3154,21 +3160,16 @@ public final class BluetoothAdapter { } } - @SuppressLint("AndroidFrameworkBluetoothPermission") - private final IBluetoothManagerCallback mManagerCallback = + private static final IBluetoothManagerCallback sManagerCallback = new IBluetoothManagerCallback.Stub() { - @SuppressLint("AndroidFrameworkRequiresPermission") public void onBluetoothServiceUp(IBluetooth bluetoothService) { if (DBG) { Log.d(TAG, "onBluetoothServiceUp: " + bluetoothService); } - mServiceLock.writeLock().lock(); - mService = bluetoothService; - mServiceLock.writeLock().unlock(); - - synchronized (mProxyServiceStateCallbacks) { - for (IBluetoothManagerCallback cb : mProxyServiceStateCallbacks) { + synchronized (sServiceLock) { + sService = bluetoothService; + for (IBluetoothManagerCallback cb : sProxyServiceStateCallbacks.keySet()) { try { if (cb != null) { cb.onBluetoothServiceUp(bluetoothService); @@ -3180,6 +3181,56 @@ public final class BluetoothAdapter { } } } + } + + public void onBluetoothServiceDown() { + if (DBG) { + Log.d(TAG, "onBluetoothServiceDown"); + } + + synchronized (sServiceLock) { + sService = null; + for (IBluetoothManagerCallback cb : sProxyServiceStateCallbacks.keySet()) { + try { + if (cb != null) { + cb.onBluetoothServiceDown(); + } else { + Log.d(TAG, "onBluetoothServiceDown: cb is null!"); + } + } catch (Exception e) { + Log.e(TAG, "", e); + } + } + } + } + + public void onBrEdrDown() { + if (VDBG) { + Log.i(TAG, "onBrEdrDown"); + } + + synchronized (sServiceLock) { + for (IBluetoothManagerCallback cb : sProxyServiceStateCallbacks.keySet()) { + try { + if (cb != null) { + cb.onBrEdrDown(); + } else { + Log.d(TAG, "onBrEdrDown: cb is null!"); + } + } catch (Exception e) { + Log.e(TAG, "", e); + } + } + } + } + }; + + private final IBluetoothManagerCallback mManagerCallback = + new IBluetoothManagerCallback.Stub() { + public void onBluetoothServiceUp(IBluetooth bluetoothService) { + synchronized (mServiceLock.writeLock()) { + mService = bluetoothService; + } synchronized (mMetadataListeners) { mMetadataListeners.forEach((device, pair) -> { try { @@ -3204,12 +3255,7 @@ public final class BluetoothAdapter { } public void onBluetoothServiceDown() { - if (DBG) { - Log.d(TAG, "onBluetoothServiceDown: " + mService); - } - - try { - mServiceLock.writeLock().lock(); + synchronized (mServiceLock.writeLock()) { mService = null; if (mLeScanClients != null) { mLeScanClients.clear(); @@ -3220,29 +3266,10 @@ public final class BluetoothAdapter { if (mBluetoothLeScanner != null) { mBluetoothLeScanner.cleanup(); } - } finally { - mServiceLock.writeLock().unlock(); - } - - synchronized (mProxyServiceStateCallbacks) { - for (IBluetoothManagerCallback cb : mProxyServiceStateCallbacks) { - try { - if (cb != null) { - cb.onBluetoothServiceDown(); - } else { - Log.d(TAG, "onBluetoothServiceDown: cb is null!"); - } - } catch (Exception e) { - Log.e(TAG, "", e); - } - } } } public void onBrEdrDown() { - if (VDBG) { - Log.i(TAG, "onBrEdrDown: " + mService); - } } }; @@ -3477,15 +3504,12 @@ public final class BluetoothAdapter { protected void finalize() throws Throwable { try { - mManagerService.unregisterAdapter(mManagerCallback); - } catch (RemoteException e) { - Log.e(TAG, "", e); + removeServiceStateCallback(mManagerCallback); } finally { super.finalize(); } } - /** * Validate a String Bluetooth address, such as "00:43:A8:23:10:F0" * <p>Alphabetic characters must be uppercase to be valid. @@ -3549,24 +3573,64 @@ public final class BluetoothAdapter { return mAttributionSource; } - private final ArrayList<IBluetoothManagerCallback> mProxyServiceStateCallbacks = - new ArrayList<IBluetoothManagerCallback>(); + @GuardedBy("sServiceLock") + private static final WeakHashMap<IBluetoothManagerCallback, Void> sProxyServiceStateCallbacks = + new WeakHashMap<>(); + + /*package*/ IBluetooth getBluetoothService() { + synchronized (sServiceLock) { + if (sProxyServiceStateCallbacks.isEmpty()) { + throw new IllegalStateException( + "Anonymous service access requires at least one lifecycle in process"); + } + return sService; + } + } @UnsupportedAppUsage /*package*/ IBluetooth getBluetoothService(IBluetoothManagerCallback cb) { - synchronized (mProxyServiceStateCallbacks) { - if (cb == null) { - Log.w(TAG, "getBluetoothService() called with no BluetoothManagerCallback"); - } else if (!mProxyServiceStateCallbacks.contains(cb)) { - mProxyServiceStateCallbacks.add(cb); - } + Objects.requireNonNull(cb); + synchronized (sServiceLock) { + sProxyServiceStateCallbacks.put(cb, null); + registerOrUnregisterAdapterLocked(); + return sService; } - return mService; } /*package*/ void removeServiceStateCallback(IBluetoothManagerCallback cb) { - synchronized (mProxyServiceStateCallbacks) { - mProxyServiceStateCallbacks.remove(cb); + Objects.requireNonNull(cb); + synchronized (sServiceLock) { + sProxyServiceStateCallbacks.remove(cb); + registerOrUnregisterAdapterLocked(); + } + } + + /** + * Handle registering (or unregistering) a single process-wide + * {@link IBluetoothManagerCallback} based on the presence of local + * {@link #sProxyServiceStateCallbacks} clients. + */ + @GuardedBy("sServiceLock") + private void registerOrUnregisterAdapterLocked() { + final boolean isRegistered = sServiceRegistered; + final boolean wantRegistered = !sProxyServiceStateCallbacks.isEmpty(); + + if (isRegistered != wantRegistered) { + if (wantRegistered) { + try { + sService = mManagerService.registerAdapter(sManagerCallback); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } else { + try { + mManagerService.unregisterAdapter(sManagerCallback); + sService = null; + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + sServiceRegistered = wantRegistered; } } diff --git a/core/java/android/bluetooth/BluetoothSocket.java b/core/java/android/bluetooth/BluetoothSocket.java index bb409d5360f9..1655b62bbfec 100644 --- a/core/java/android/bluetooth/BluetoothSocket.java +++ b/core/java/android/bluetooth/BluetoothSocket.java @@ -399,7 +399,7 @@ public final class BluetoothSocket implements Closeable { try { if (mSocketState == SocketState.CLOSED) throw new IOException("socket closed"); IBluetooth bluetoothProxy = - BluetoothAdapter.getDefaultAdapter().getBluetoothService(null); + BluetoothAdapter.getDefaultAdapter().getBluetoothService(); if (bluetoothProxy == null) throw new IOException("Bluetooth is off"); mPfd = bluetoothProxy.getSocketManager().connectSocket(mDevice, mType, mUuid, mPort, getSecurityFlags()); @@ -438,7 +438,7 @@ public final class BluetoothSocket implements Closeable { /*package*/ int bindListen() { int ret; if (mSocketState == SocketState.CLOSED) return EBADFD; - IBluetooth bluetoothProxy = BluetoothAdapter.getDefaultAdapter().getBluetoothService(null); + IBluetooth bluetoothProxy = BluetoothAdapter.getDefaultAdapter().getBluetoothService(); if (bluetoothProxy == null) { Log.e(TAG, "bindListen fail, reason: bluetooth is off"); return -1; @@ -706,7 +706,7 @@ public final class BluetoothSocket implements Closeable { throw new IOException("socket closed"); } IBluetooth bluetoothProxy = - BluetoothAdapter.getDefaultAdapter().getBluetoothService(null); + BluetoothAdapter.getDefaultAdapter().getBluetoothService(); if (bluetoothProxy == null) { throw new IOException("Bluetooth is off"); } |
