Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5e9f4a0e authored by William Escande's avatar William Escande
Browse files

SystemServer: Remove un-necessary mAdapterLock

The lock is legacy. It comes from a times where there was multiples
thread that could update the mAdapter. This is no longer the case and
the only other thread that interact with mAdapter beside the main looper
is the binder thread.
The access to mAdapter from the binder thread is copying the value to
prevent asynchrone null set

Bug: 311772251
Test: atest CtsBluetoothTestCases
Test: atest pts-bot
Flag: Exempt refactor without logical change
Change-Id: I629c9992ef0d9849ad1301d45d127eaef7f1cb30
parent 010483ed
Loading
Loading
Loading
Loading
+233 −349
Original line number Diff line number Diff line
@@ -84,7 +84,6 @@ import android.util.proto.ProtoOutputStream;
import androidx.annotation.RequiresApi;

import com.android.bluetooth.flags.Flags;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.modules.expresslog.Counter;
import com.android.modules.expresslog.Histogram;
@@ -116,7 +115,6 @@ import java.util.concurrent.Executors;
import java.util.concurrent.FutureTask;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.locks.ReentrantReadWriteLock;

class BluetoothManagerService {
    private static final String TAG = BluetoothManagerService.class.getSimpleName();
@@ -188,9 +186,6 @@ class BluetoothManagerService {
            new RemoteCallbackList<IBluetoothManagerCallback>();
    private final BluetoothServiceBinder mBinder;

    private final ReentrantReadWriteLock mAdapterLock = new ReentrantReadWriteLock();

    @GuardedBy("mAdapterLock")
    private AdapterBinder mAdapter = null;

    // used inside handler thread
@@ -267,11 +262,6 @@ class BluetoothManagerService {
        // Clear registered LE apps to force shut-off Bluetooth
        clearBleApps();
        int state = getState();
        mAdapterLock.readLock().lock();
        try {
            if (mAdapter == null) {
                return false;
            }
        if (state == STATE_BLE_ON) {
            ActiveLogs.add(ENABLE_DISABLE_REASON_FACTORY_RESET, false);
            bleOnToOff();
@@ -281,9 +271,6 @@ class BluetoothManagerService {
            onToBleOn();
            return true;
        }
        } finally {
            mAdapterLock.readLock().unlock();
        }
        return false;
    }

@@ -405,31 +392,15 @@ class BluetoothManagerService {
        }

        if (currentState == STATE_ON) {
            mAdapterLock.readLock().lock();
            try {
                if (mAdapter == null) {
                    return;
                }
            mEnable = false;
            ActiveLogs.add(reason, false);
            onToBleOn();
            } finally {
                mAdapterLock.readLock().unlock();
            }
        } else if (currentState == STATE_BLE_ON) {
            // If currentState is BLE_ON make sure we trigger stopBle
            mAdapterLock.readLock().lock();
            try {
                if (mAdapter == null) {
                    return;
                }
            mEnable = false;
            mEnableExternal = false;
            ActiveLogs.add(reason, false);
            bleOnToOff();
            } finally {
                mAdapterLock.readLock().unlock();
            }
        }
    }

@@ -544,21 +515,13 @@ class BluetoothManagerService {
                    } else if (action.equals(Intent.ACTION_SHUTDOWN)) {
                        Log.i(TAG, "Device is shutting down.");
                        mShutdownInProgress = true;
                        mAdapterLock.readLock().lock();
                        try {
                        mEnable = false;
                        mEnableExternal = false;
                            if (mAdapter == null) {
                                return;
                            }
                        if (mState.oneOf(STATE_BLE_ON)) {
                            bleOnToOff();
                        } else if (mState.oneOf(STATE_ON)) {
                            onToBleOn();
                        }
                        } finally {
                            mAdapterLock.readLock().unlock();
                        }
                    }
                }
            };
@@ -781,11 +744,14 @@ class BluetoothManagerService {
        Log.d(TAG, logHeader + "Completed successfully");
    }

    // Called from unsafe binder thread
    IBluetooth registerAdapter(IBluetoothManagerCallback callback) {
        synchronized (mCallbacks) {
            mCallbacks.register(callback);
        }
        return mAdapter != null ? mAdapter.getAdapterBinder() : null;
        // Copy to local variable to avoid race condition when checking for null
        AdapterBinder adapter = mAdapter;
        return adapter != null ? adapter.getAdapterBinder() : null;
    }

    void unregisterAdapter(IBluetoothManagerCallback callback) {
@@ -805,15 +771,13 @@ class BluetoothManagerService {
     * @param userId is the foreground user id we are propagating to the Bluetooth process
     */
    private void propagateForegroundUserId(int userId) {
        mAdapterLock.readLock().lock();
        if (mAdapter == null) {
            return;
        }
        try {
            if (mAdapter != null) {
            mAdapter.setForegroundUserId(userId, mContext.getAttributionSource());
            }
        } catch (RemoteException e) {
            Log.e(TAG, "Unable to set foreground user id", e);
        } finally {
            mAdapterLock.readLock().unlock();
        }
    }

@@ -875,7 +839,7 @@ class BluetoothManagerService {
    }

    boolean isMediaProfileConnected() {
        if (mAdapter == null || !mState.oneOf(STATE_ON)) {
        if (!mState.oneOf(STATE_ON)) {
            return false;
        }
        return mAdapter.isMediaProfileConnected(mContext.getAttributionSource());
@@ -894,15 +858,10 @@ class BluetoothManagerService {
                        // BLE scan is not available.
                        disableBleScanMode();
                        clearBleApps();
                        mAdapterLock.readLock().lock();
                        try {
                            if (mAdapter != null) {
                        if (mState.oneOf(STATE_BLE_ON)) {
                            ActiveLogs.add(ENABLE_DISABLE_REASON_APPLICATION_REQUEST, false);
                            bleOnToOff();
                        }
                        } finally {
                            mAdapterLock.readLock().unlock();
                        }
                    }
                };

@@ -914,15 +873,10 @@ class BluetoothManagerService {

    // Disable ble scan only mode.
    private void disableBleScanMode() {
        mAdapterLock.writeLock().lock();
        try {
            if (mAdapter != null && mState.oneOf(STATE_ON)) {
        if (mState.oneOf(STATE_ON)) {
            Log.d(TAG, "disableBleScanMode: Resetting the mEnable flag for clean disable");
            mEnable = false;
        }
        } finally {
            mAdapterLock.writeLock().unlock();
        }
    }

    private int updateBleAppCount(IBinder token, boolean enable, String packageName) {
@@ -1045,10 +999,8 @@ class BluetoothManagerService {
     * BLE should be off
     */
    private void continueFromBleOnState() {
        mAdapterLock.readLock().lock();
        try {
            if (mAdapter == null) {
                Log.e(TAG, "continueFromBleOnState: Adapter is null");
        if (!mState.oneOf(STATE_BLE_ON)) {
            Log.e(TAG, "continueFromBleOnState: Impossible transition from " + mState);
            return;
        }
        if (!mEnableExternal && !isBleAppPresent()) {
@@ -1067,9 +1019,6 @@ class BluetoothManagerService {
        } else {
            Log.i(TAG, "continueFromBleOnState: Staying in BLE_ON");
        }
        } finally {
            mAdapterLock.readLock().unlock();
        }
    }

    /**
@@ -1077,10 +1026,8 @@ class BluetoothManagerService {
     * if no LE app needs it
     */
    private void sendBrEdrDownCallback() {
        mAdapterLock.readLock().lock();
        try {
        if (mAdapter == null) {
                Log.w(TAG, "sendBrEdrDownCallback: mAdapter is null");
            Log.d(TAG, "sendBrEdrDownCallback: mAdapter is null");
            return;
        }
        boolean scanIsAllowed =
@@ -1088,16 +1035,15 @@ class BluetoothManagerService {
        if (!AirplaneModeListener.isOn() && isBleAppPresent() && scanIsAllowed) {
            // Need to stay at BLE ON. Disconnect all Gatt connections
            Log.i(TAG, "sendBrEdrDownCallback: Staying in BLE_ON");
            try {
                mAdapter.unregAllGattClient(mContext.getAttributionSource());
            } catch (RemoteException e) {
                Log.e(TAG, "sendBrEdrDownCallback: failed to call unregAllGattClient()", e);
            }
        } else {
            Log.i(TAG, "sendBrEdrDownCallback: Stopping ble");
            bleOnToOff();
        }
        } catch (RemoteException e) {
            Log.e(TAG, "sendBrEdrDownCallback: Call to mAdapter failed.", e);
        } finally {
            mAdapterLock.readLock().unlock();
        }
    }

    private Unit enableFromAutoOn() {
@@ -1187,18 +1133,13 @@ class BluetoothManagerService {
    void unbindAndFinish() {
        Log.d(TAG, "unbindAndFinish(): mAdapter=" + mAdapter + " isBinding=" + isBinding());

        mAdapterLock.writeLock().lock();
        try {
        mHandler.removeMessages(MESSAGE_BLUETOOTH_STATE_CHANGE);
            if (mAdapter == null) {
                return;
            }
        long currentTimeMs = System.currentTimeMillis();

        try {
            mAdapter.unregisterCallback(mBluetoothCallback, mContext.getAttributionSource());
        } catch (RemoteException e) {
                Log.e(TAG, "Unable to unregister BluetoothCallback", e);
            Log.e(TAG, "unbindAndFinish(): Unable to unregister BluetoothCallback", e);
        }

        CompletableFuture<Void> binderDead = new CompletableFuture<>();
@@ -1212,7 +1153,7 @@ class BluetoothManagerService {
                            },
                            0);
        } catch (RemoteException e) {
                Log.e(TAG, "Failed to linkToDeath", e);
            Log.e(TAG, "unbindAndFinish(): Failed to linkToDeath", e);
            binderDead.complete(null);
        }

@@ -1229,11 +1170,11 @@ class BluetoothManagerService {
            binderDead.get(2_000, TimeUnit.MILLISECONDS);
        } catch (android.os.DeadObjectException e) {
            // Reduce exception to info and skip waiting (Bluetooth is dead as wanted)
                Log.i(TAG, "Bluetooth already dead 💀");
            Log.i(TAG, "unbindAndFinish(): Bluetooth already dead 💀");
        } catch (RemoteException e) {
                Log.e(TAG, "Unexpected RemoteException when calling killBluetoothProcess", e);
            Log.e(TAG, "unbindAndFinish(): Unable to call killBluetoothProcess", e);
        } catch (TimeoutException | InterruptedException | ExecutionException e) {
                Log.e(TAG, "Bluetooth death not received correctly after > 2000ms", e);
            Log.e(TAG, "unbindAndFinish(): Bluetooth death not received after > 2000ms", e);
        }

        long timeSpentForShutdown = System.currentTimeMillis() - currentTimeMs;
@@ -1245,9 +1186,6 @@ class BluetoothManagerService {

        mAdapter = null;
        mHandler.removeMessages(MESSAGE_TIMEOUT_BIND);
        } finally {
            mAdapterLock.writeLock().unlock();
        }
    }

    /**
@@ -1349,8 +1287,6 @@ class BluetoothManagerService {
    /** Inform BluetoothAdapter instances that Adapter service is up */
    private void sendBluetoothServiceUpCallback() {
        synchronized (mCallbacks) {
            mAdapterLock.readLock().lock();
            try {
            int n = mCallbacks.beginBroadcast();
            Log.d(TAG, "sendBluetoothServiceUpCallback(): to " + n + " receivers");
            for (int i = 0; i < n; i++) {
@@ -1362,17 +1298,13 @@ class BluetoothManagerService {
                    Log.e(TAG, "Unable to call onBluetoothServiceUp() on callback #" + i, e);
                }
            }
            } finally {
            mCallbacks.finishBroadcast();
                mAdapterLock.readLock().unlock();
            }
        }
    }

    /** Inform BluetoothAdapter instances that Adapter service is down */
    private void sendBluetoothServiceDownCallback() {
        synchronized (mCallbacks) {
            try {
            int n = mCallbacks.beginBroadcast();
            Log.d(TAG, "sendBluetoothServiceDownCallback(): to " + n + " receivers");
            for (int i = 0; i < n; i++) {
@@ -1382,25 +1314,20 @@ class BluetoothManagerService {
                    Log.e(TAG, "Unable to call onBluetoothServiceDown() on callback #" + i, e);
                }
            }
            } finally {
            mCallbacks.finishBroadcast();
        }
    }
    }

    // Called from unsafe binder thread
    String getAddress() {
        mAdapterLock.readLock().lock();
        // Copy to local variable to avoid race condition when checking for null
        AdapterBinder adapter = mAdapter;
        if (adapter != null) {
            try {
            if (mAdapter != null) {
                return mAdapter.getAddress(mContext.getAttributionSource());
            }
            } catch (RemoteException e) {
            Log.e(
                    TAG,
                    "getAddress(): Unable to retrieve address remotely. Returning cached address",
                    e);
        } finally {
            mAdapterLock.readLock().unlock();
                Log.e(TAG, "getAddress(): Returning cached address", e);
            }
        }

        // mAddress is accessed from outside.
@@ -1409,16 +1336,16 @@ class BluetoothManagerService {
        return mAddress;
    }

    // Called from unsafe binder thread
    String getName() {
        mAdapterLock.readLock().lock();
        // Copy to local variable to avoid race condition when checking for null
        AdapterBinder adapter = mAdapter;
        if (adapter != null) {
            try {
            if (mAdapter != null) {
                return mAdapter.getName(mContext.getAttributionSource());
            }
            } catch (RemoteException e) {
            Log.e(TAG, "getName(): Unable to retrieve name remotely. Returning cached name", e);
        } finally {
            mAdapterLock.readLock().unlock();
                Log.e(TAG, "getName(): Returning cached name", e);
            }
        }

        // mName is accessed from outside.
@@ -1468,8 +1395,6 @@ class BluetoothManagerService {
            switch (msg.what) {
                case MESSAGE_GET_NAME_AND_ADDRESS:
                    Log.d(TAG, "MESSAGE_GET_NAME_AND_ADDRESS");
                    mAdapterLock.writeLock().lock();
                    try {
                    if (mAdapter == null && !isBinding()) {
                        Log.d(TAG, "Binding to service to get name and address");
                        mGetNameAddressOnly = true;
@@ -1480,16 +1405,13 @@ class BluetoothManagerService {
                                    mAdapter.getName(mContext.getAttributionSource()),
                                    mAdapter.getAddress(mContext.getAttributionSource()));
                        } catch (RemoteException e) {
                                Log.e(TAG, "Unable to grab names", e);
                            Log.e(TAG, "Unable to grab name or address", e);
                        }
                        if (mGetNameAddressOnly && !mEnable) {
                            unbindAndFinish();
                        }
                        mGetNameAddressOnly = false;
                    }
                    } finally {
                        mAdapterLock.writeLock().unlock();
                    }
                    break;

                case MESSAGE_ENABLE:
@@ -1600,15 +1522,12 @@ class BluetoothManagerService {
                    IBinder service = (IBinder) msg.obj;
                    Log.d(TAG, "MESSAGE_BLUETOOTH_SERVICE_CONNECTED: service=" + service);

                    mAdapterLock.writeLock().lock();
                    try {
                    // Remove timeout
                    mHandler.removeMessages(MESSAGE_TIMEOUT_BIND);

                    mAdapter = BluetoothServerProxy.getInstance().createAdapterBinder(service);

                        int foregroundUserId = ActivityManager.getCurrentUser();
                        propagateForegroundUserId(foregroundUserId);
                    propagateForegroundUserId(ActivityManager.getCurrentUser());

                    if (!isNameAndAddressSet()) {
                        mHandler.sendEmptyMessage(MESSAGE_GET_NAME_AND_ADDRESS);
@@ -1634,9 +1553,6 @@ class BluetoothManagerService {
                    if (Flags.fastBindToApp()) {
                        sendBluetoothServiceUpCallback();
                    }
                    } finally {
                        mAdapterLock.writeLock().unlock();
                    }

                    if (!mEnable) {
                        waitForState(STATE_ON);
@@ -1657,13 +1573,11 @@ class BluetoothManagerService {
                    // unbind and rebind bluetooth service and enable bluetooth
                    if ((prevState == STATE_BLE_TURNING_ON)
                            && (newState == STATE_OFF)
                            && (mAdapter != null)
                            && mEnable) {
                        recoverBluetoothServiceFromError(false);
                    }
                    if ((prevState == STATE_TURNING_ON)
                            && (newState == STATE_BLE_ON)
                            && (mAdapter != null)
                            && mEnable) {
                        recoverBluetoothServiceFromError(true);
                    }
@@ -1689,17 +1603,12 @@ class BluetoothManagerService {

                case MESSAGE_BLUETOOTH_SERVICE_DISCONNECTED:
                    Log.e(TAG, "MESSAGE_BLUETOOTH_SERVICE_DISCONNECTED");
                    mAdapterLock.writeLock().lock();
                    try {
                    // if service is unbinded already, do nothing and return
                    if (mAdapter == null) {
                        break;
                    }
                    mContext.unbindService(mConnection);
                    mAdapter = null;
                    } finally {
                        mAdapterLock.writeLock().unlock();
                    }

                    // log the unexpected crash
                    addCrashLog();
@@ -1739,9 +1648,12 @@ class BluetoothManagerService {
                        ActiveLogs.add(ENABLE_DISABLE_REASON_RESTARTED, true);
                        handleEnable(mQuietEnable);
                    } else {
                        mAdapterLock.writeLock().lock();
                        // if service is unbinded already, do nothing and return
                        if (mAdapter == null) {
                            break;
                        }
                        mContext.unbindService(mConnection);
                        mAdapter = null;
                        mAdapterLock.writeLock().unlock();
                        Log.e(TAG, "Reach maximum retry to restart Bluetooth!");
                    }
                    break;
@@ -1762,7 +1674,7 @@ class BluetoothManagerService {
                    mCurrentUserContext = mContext.createContextAsUser(userTo, 0);

                    /* disable and enable BT when detect a user switch */
                    if (mAdapter != null && mState.oneOf(STATE_ON)) {
                    if (mState.oneOf(STATE_ON)) {
                        restartForNewUser(userTo);
                    } else if (isBinding() || mAdapter != null) {
                        Message userMsg = Message.obtain(msg);
@@ -1797,21 +1709,12 @@ class BluetoothManagerService {
        }

        private void restartForNewUser(UserHandle unusedNewUser) {
            mAdapterLock.readLock().lock();
            try {
                if (mAdapter != null) {
                    mAdapter.unregisterCallback(
                            mBluetoothCallback, mContext.getAttributionSource());
                }
                mAdapter.unregisterCallback(mBluetoothCallback, mContext.getAttributionSource());
            } catch (RemoteException e) {
                Log.e(TAG, "Unable to unregister", e);
            } finally {
                mAdapterLock.readLock().unlock();
            }

            // This method is always called while bluetooth is in STATE_ON
            assert (mState.oneOf(STATE_ON));

            // disable
            ActiveLogs.add(ENABLE_DISABLE_REASON_USER_SWITCH, false);
            onToBleOn();
@@ -1870,9 +1773,6 @@ class BluetoothManagerService {
            setBluetoothPersistedState(BLUETOOTH_ON_BLUETOOTH);
        }

        // Use service interface to get the exact state
        mAdapterLock.readLock().lock();
        try {
        if (mAdapter != null) {
            boolean isHandled = true;
            switch (mState.get()) {
@@ -1895,9 +1795,6 @@ class BluetoothManagerService {
            }
            if (isHandled) return;
        }
        } finally {
            mAdapterLock.readLock().unlock();
        }

        mQuietEnable = (quietEnable == 1);
        if (mAdapter == null) {
@@ -1969,17 +1866,12 @@ class BluetoothManagerService {
    private void handleEnable(boolean quietMode) {
        mQuietEnable = quietMode;

        mAdapterLock.writeLock().lock();
        try {
        if (mAdapter == null && !isBinding()) {
            bindToAdapter();
        } else if (!Flags.fastBindToApp() && mAdapter != null) {
            // Enable bluetooth
            offToBleOn();
        }
        } finally {
            mAdapterLock.writeLock().unlock();
        }
    }

    private void offToBleOn() {
@@ -2157,16 +2049,13 @@ class BluetoothManagerService {
            mHandler.removeCallbacksAndMessages(ON_AIRPLANE_MODE_CHANGED_TOKEN);
            repeatAirplaneRunnable = true;
        }
        mAdapterLock.readLock().lock();
        try {

        if (mAdapter != null) {
                // Unregister callback object
            try {
                mAdapter.unregisterCallback(mBluetoothCallback, mContext.getAttributionSource());
            }
            } catch (RemoteException e) {
                Log.e(TAG, "Unable to unregister", e);
        } finally {
            mAdapterLock.readLock().unlock();
            }
        }

        Log.d(TAG, "Force sleep 500 ms for recovering from error");
@@ -2180,16 +2069,11 @@ class BluetoothManagerService {

        sendBluetoothServiceDownCallback();

        mAdapterLock.writeLock().lock();
        try {
        if (mAdapter != null) {
            mAdapter = null;
            // Unbind
            mContext.unbindService(mConnection);
        }
        } finally {
            mAdapterLock.writeLock().unlock();
        }

        mHandler.removeMessages(MESSAGE_BLUETOOTH_STATE_CHANGE);
        mState.set(STATE_OFF);