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

Commit 5312af0c authored by Jaikumar Ganesh's avatar Jaikumar Ganesh
Browse files

Fix excessive locking synchronization leading to deadlocks. 

We should never have to look on the entire instance. There
are many places in the code where it there and we need to fix it.
This is a start. In this particular case what we are protecting
is the properties map of the remote device and thus we just need to lock
on it. Ofcourse, it would be better to have a state machine but this
is good for now.

Deadlock: When a new device was found it will call addProperties which will hold
a lock. addProperties calls into BluetoothService. Now Settings app makes a call
into BluetoothService which will call into this file and we have a deadlock.

Change-Id: Ieb69d5ace222bf5d1e6677af151241153303099f
parent 7656b21e

core/java/android/server/BluetoothDeviceProperties.java

+62 −50
Original line number Diff line number Diff line
@@ -35,13 +35,14 @@ class BluetoothDeviceProperties { 
        mService = service;

    }



    synchronized Map<String, String> addProperties(String address,

            String[] properties) {

    Map<String, String> addProperties(String address, String[] properties) {

        /*

         * We get a DeviceFound signal every time RSSI changes or name changes.

         * Don't create a new Map object every time.

         */

        Map<String, String> propertyValues = mPropertiesMap.get(address);

        Map<String, String> propertyValues;

        synchronized(mPropertiesMap) {

            propertyValues = mPropertiesMap.get(address);

            if (propertyValues == null) {

                propertyValues = new HashMap<String, String>();

            }
@@ -72,6 +73,7 @@ class BluetoothDeviceProperties { 
                propertyValues.put(name, newValue);

            }

            mPropertiesMap.put(address, propertyValues);

        }



        // We have added a new remote device or updated its properties.

        // Also update the serviceChannel cache.
@@ -79,7 +81,8 @@ class BluetoothDeviceProperties { 
        return propertyValues;

    }



    synchronized void setProperty(String address, String name, String value) {

    void setProperty(String address, String name, String value) {

        synchronized(mPropertiesMap) {

            Map <String, String> propVal = mPropertiesMap.get(address);

            if (propVal != null) {

                propVal.put(name, value);
@@ -88,20 +91,28 @@ class BluetoothDeviceProperties { 
                Log.e(TAG, "setRemoteDeviceProperty for a device not in cache:" + address);

            }

        }

    }



    synchronized boolean isInCache(String address) {

    boolean isInCache(String address) {

        synchronized (mPropertiesMap) {

            return (mPropertiesMap.get(address) != null);

        }

    }



    synchronized boolean isEmpty() {

    boolean isEmpty() {

        synchronized (mPropertiesMap) {

            return mPropertiesMap.isEmpty();

        }

    }



    synchronized Set<String> keySet() {

    Set<String> keySet() {

        synchronized (mPropertiesMap) {

            return mPropertiesMap.keySet();

        }

    }



    synchronized String getProperty(String address, String property) {

    String getProperty(String address, String property) {

        synchronized(mPropertiesMap) {

            Map<String, String> properties = mPropertiesMap.get(address);

            if (properties != null) {

                return properties.get(property);
@@ -114,11 +125,12 @@ class BluetoothDeviceProperties { 
                    return properties.get(property);

                }

            }

        }

        Log.e(TAG, "getRemoteDeviceProperty: " + property + " not present: " + address);

        return null;

    }



    synchronized Map<String, String> updateCache(String address) {

    Map<String, String> updateCache(String address) {

        String[] propValues = mService.getRemoteDeviceProperties(address);

        if (propValues != null) {

            return addProperties(address, propValues);