From 8175bf2b80ccfffac32e2dff1d8f69ce90cddd15 Mon Sep 17 00:00:00 2001 From: hughchen Date: Wed, 12 Feb 2020 18:52:34 +0800 Subject: [PATCH] Use CopyOnWriteArrayList to avoid ConcurrentModificationException Bug: 149362541 Test: make -j42 RunSettingsLibRoboTests Change-Id: I9bed14c0141a896ec91d2354df09bb5aaa432dfb --- .../settingslib/media/LocalMediaManager.java | 35 +++++------- .../settingslib/media/MediaManager.java | 55 ++++++++----------- 2 files changed, 37 insertions(+), 53 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/media/LocalMediaManager.java b/packages/SettingsLib/src/com/android/settingslib/media/LocalMediaManager.java index a1342ecdcfa7b..984ab11a1a712 100644 --- a/packages/SettingsLib/src/com/android/settingslib/media/LocalMediaManager.java +++ b/packages/SettingsLib/src/com/android/settingslib/media/LocalMediaManager.java @@ -35,6 +35,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; /** * LocalMediaManager provide interface to get MediaDevice list and transfer media to MediaDevice. @@ -53,7 +54,7 @@ public class LocalMediaManager implements BluetoothCallback { int STATE_DISCONNECTED = 3; } - private final Collection mCallbacks = new ArrayList<>(); + private final Collection mCallbacks = new CopyOnWriteArrayList<>(); @VisibleForTesting final MediaDeviceCallback mMediaDeviceCallback = new MediaDeviceCallback(); @@ -73,18 +74,14 @@ public class LocalMediaManager implements BluetoothCallback { * Register to start receiving callbacks for MediaDevice events. */ public void registerCallback(DeviceCallback callback) { - synchronized (mCallbacks) { - mCallbacks.add(callback); - } + mCallbacks.add(callback); } /** * Unregister to stop receiving callbacks for MediaDevice events */ public void unregisterCallback(DeviceCallback callback) { - synchronized (mCallbacks) { - mCallbacks.remove(callback); - } + mCallbacks.remove(callback); } public LocalMediaManager(Context context, String packageName, Notification notification) { @@ -152,10 +149,8 @@ public class LocalMediaManager implements BluetoothCallback { } void dispatchSelectedDeviceStateChanged(MediaDevice device, @MediaDeviceState int state) { - synchronized (mCallbacks) { - for (DeviceCallback callback : mCallbacks) { - callback.onSelectedDeviceStateChanged(device, state); - } + for (DeviceCallback callback : getCallbacks()) { + callback.onSelectedDeviceStateChanged(device, state); } } @@ -169,19 +164,15 @@ public class LocalMediaManager implements BluetoothCallback { } void dispatchDeviceListUpdate() { - synchronized (mCallbacks) { - Collections.sort(mMediaDevices, COMPARATOR); - for (DeviceCallback callback : mCallbacks) { - callback.onDeviceListUpdate(new ArrayList<>(mMediaDevices)); - } + Collections.sort(mMediaDevices, COMPARATOR); + for (DeviceCallback callback : getCallbacks()) { + callback.onDeviceListUpdate(new ArrayList<>(mMediaDevices)); } } void dispatchDeviceAttributesChanged() { - synchronized (mCallbacks) { - for (DeviceCallback callback : mCallbacks) { - callback.onDeviceAttributesChanged(); - } + for (DeviceCallback callback : getCallbacks()) { + callback.onDeviceAttributesChanged(); } } @@ -270,6 +261,10 @@ public class LocalMediaManager implements BluetoothCallback { || device.isActiveDevice(BluetoothProfile.HEARING_AID); } + private Collection getCallbacks() { + return new CopyOnWriteArrayList<>(mCallbacks); + } + class MediaDeviceCallback implements MediaManager.MediaDeviceCallback { @Override public void onDeviceAdded(MediaDevice device) { diff --git a/packages/SettingsLib/src/com/android/settingslib/media/MediaManager.java b/packages/SettingsLib/src/com/android/settingslib/media/MediaManager.java index 7898982bccbc8..73551f60c462b 100644 --- a/packages/SettingsLib/src/com/android/settingslib/media/MediaManager.java +++ b/packages/SettingsLib/src/com/android/settingslib/media/MediaManager.java @@ -22,6 +22,7 @@ import android.util.Log; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; /** * MediaManager provide interface to get MediaDevice list. @@ -30,7 +31,7 @@ public abstract class MediaManager { private static final String TAG = "MediaManager"; - protected final Collection mCallbacks = new ArrayList<>(); + protected final Collection mCallbacks = new CopyOnWriteArrayList<>(); protected final List mMediaDevices = new ArrayList<>(); protected Context mContext; @@ -42,18 +43,14 @@ public abstract class MediaManager { } protected void registerCallback(MediaDeviceCallback callback) { - synchronized (mCallbacks) { - if (!mCallbacks.contains(callback)) { - mCallbacks.add(callback); - } + if (!mCallbacks.contains(callback)) { + mCallbacks.add(callback); } } protected void unregisterCallback(MediaDeviceCallback callback) { - synchronized (mCallbacks) { - if (mCallbacks.contains(callback)) { - mCallbacks.remove(callback); - } + if (mCallbacks.contains(callback)) { + mCallbacks.remove(callback); } } @@ -78,53 +75,45 @@ public abstract class MediaManager { } protected void dispatchDeviceAdded(MediaDevice mediaDevice) { - synchronized (mCallbacks) { - for (MediaDeviceCallback callback : mCallbacks) { - callback.onDeviceAdded(mediaDevice); - } + for (MediaDeviceCallback callback : getCallbacks()) { + callback.onDeviceAdded(mediaDevice); } } protected void dispatchDeviceRemoved(MediaDevice mediaDevice) { - synchronized (mCallbacks) { - for (MediaDeviceCallback callback : mCallbacks) { - callback.onDeviceRemoved(mediaDevice); - } + for (MediaDeviceCallback callback : getCallbacks()) { + callback.onDeviceRemoved(mediaDevice); } } protected void dispatchDeviceListAdded() { - synchronized (mCallbacks) { - for (MediaDeviceCallback callback : mCallbacks) { - callback.onDeviceListAdded(new ArrayList<>(mMediaDevices)); - } + for (MediaDeviceCallback callback : getCallbacks()) { + callback.onDeviceListAdded(new ArrayList<>(mMediaDevices)); } } protected void dispatchDeviceListRemoved(List devices) { - synchronized (mCallbacks) { - for (MediaDeviceCallback callback : mCallbacks) { - callback.onDeviceListRemoved(devices); - } + for (MediaDeviceCallback callback : getCallbacks()) { + callback.onDeviceListRemoved(devices); } } protected void dispatchConnectedDeviceChanged(String id) { - synchronized (mCallbacks) { - for (MediaDeviceCallback callback : mCallbacks) { - callback.onConnectedDeviceChanged(id); - } + for (MediaDeviceCallback callback : getCallbacks()) { + callback.onConnectedDeviceChanged(id); } } protected void dispatchDataChanged() { - synchronized (mCallbacks) { - for (MediaDeviceCallback callback : mCallbacks) { - callback.onDeviceAttributesChanged(); - } + for (MediaDeviceCallback callback : getCallbacks()) { + callback.onDeviceAttributesChanged(); } } + private Collection getCallbacks() { + return new CopyOnWriteArrayList<>(mCallbacks); + } + /** * Callback for notifying device is added, removed and attributes changed. */