From 77535ce22a71b04fd45f526e8c03976e42761ec2 Mon Sep 17 00:00:00 2001 From: jackqdyulei Date: Wed, 13 Feb 2019 13:17:38 -0800 Subject: [PATCH] Fix deadlock in LocalBluetoothManager Following deadlock exist in previous code: 1. Main thread Device.refresh()->Device.dispatchCallback()->Lock callback->(handle callback in systemui)->DeviceManager.getDevicesCopy()-> try lock deviceManager 2. Bg thread DeviceManager.onUuidChange()->Lock deviceManager->Device.refresh()->Device.dispatchCallback()-> try lock callback This CL remove sychnorized method in DeviceManager to unblock this deadlock. Since UuidChange can only happen in bg thread, we don't need this extra lock in DeviceManager. Fixes: 112483982 Test: Manual Change-Id: I1a8bb130cfff40358783f5f5da7e50e5f0a42e20 --- .../bluetooth/BluetoothEventManager.java | 10 ++++++++-- .../bluetooth/CachedBluetoothDeviceManager.java | 14 -------------- .../CachedBluetoothDeviceManagerTest.java | 15 --------------- 3 files changed, 8 insertions(+), 31 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/bluetooth/BluetoothEventManager.java b/packages/SettingsLib/src/com/android/settingslib/bluetooth/BluetoothEventManager.java index 4f18d450462d5..402ce90d6ec56 100644 --- a/packages/SettingsLib/src/com/android/settingslib/bluetooth/BluetoothEventManager.java +++ b/packages/SettingsLib/src/com/android/settingslib/bluetooth/BluetoothEventManager.java @@ -420,14 +420,20 @@ public class BluetoothEventManager { private class ClassChangedHandler implements Handler { public void onReceive(Context context, Intent intent, BluetoothDevice device) { - mDeviceManager.onBtClassChanged(device); + CachedBluetoothDevice cachedDevice = mDeviceManager.findDevice(device); + if (cachedDevice != null) { + cachedDevice.refresh(); + } } } private class UuidChangedHandler implements Handler { public void onReceive(Context context, Intent intent, BluetoothDevice device) { - mDeviceManager.onUuidChanged(device); + CachedBluetoothDevice cachedDevice = mDeviceManager.findDevice(device); + if (cachedDevice != null) { + cachedDevice.onUuidChanged(); + } } } diff --git a/packages/SettingsLib/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManager.java b/packages/SettingsLib/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManager.java index 5b4a8b4f259e0..33e7540448734 100644 --- a/packages/SettingsLib/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManager.java +++ b/packages/SettingsLib/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManager.java @@ -206,20 +206,6 @@ public class CachedBluetoothDeviceManager { } } - public synchronized void onBtClassChanged(BluetoothDevice device) { - CachedBluetoothDevice cachedDevice = findDevice(device); - if (cachedDevice != null) { - cachedDevice.dispatchAttributesChanged(); - } - } - - public synchronized void onUuidChanged(BluetoothDevice device) { - CachedBluetoothDevice cachedDevice = findDevice(device); - if (cachedDevice != null) { - cachedDevice.onUuidChanged(); - } - } - public synchronized void onBluetoothStateChanged(int bluetoothState) { // When Bluetooth is turning off, we need to clear the non-bonded devices // Otherwise, they end up showing up on the next BT enable diff --git a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManagerTest.java b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManagerTest.java index 43b289464e1d0..dc47de8546dbe 100644 --- a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManagerTest.java +++ b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/bluetooth/CachedBluetoothDeviceManagerTest.java @@ -368,21 +368,6 @@ public class CachedBluetoothDeviceManagerTest { verify(mHearingAidDeviceManager).updateHearingAidsDevices(); } - /** - * Test to verify onBtClassChanged(). - */ - @Test - public void onBtClassChanged_validBtClass_classChanged() { - CachedBluetoothDevice cachedDevice1 = mCachedDeviceManager.addDevice(mDevice1); - assertThat(cachedDevice1).isNotNull(); - assertThat(cachedDevice1.getBtClass()).isEqualTo(DEVICE_CLASS_1); - - final BluetoothClass newBluetoothClass = DEVICE_CLASS_2; - when(mDevice1.getBluetoothClass()).thenReturn(newBluetoothClass); - mCachedDeviceManager.onBtClassChanged(mDevice1); - assertThat(cachedDevice1.getBtClass()).isEqualTo(newBluetoothClass); - } - /** * Test to verify onDeviceDisappeared(). */