From 8c3a767b2dea8cc8fa75f39c600252e3cd1f76e9 Mon Sep 17 00:00:00 2001 From: jiabin Date: Tue, 22 May 2018 15:44:21 -0700 Subject: [PATCH] Avoid race condition when broadcasting device list changed. Since broadcastDeviceListChanged could be called in different threads, there would be race condition causing mutilple callback due to mPreviousPorts is not thread safe. Bug: 80138804 Test: run TestDeviceList app in toolbox and cts Change-Id: I0aa70dc45594bca263ea6f36703f22fe0293f679 --- media/java/android/media/AudioManager.java | 40 ++++++++++++---------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/media/java/android/media/AudioManager.java b/media/java/android/media/AudioManager.java index 3057501ea726a..6c9a01b899774 100644 --- a/media/java/android/media/AudioManager.java +++ b/media/java/android/media/AudioManager.java @@ -4717,7 +4717,7 @@ public class AudioManager { NativeEventHandlerDelegate delegate = new NativeEventHandlerDelegate(callback, handler); mDeviceCallbacks.put(callback, delegate); - broadcastDeviceListChange(delegate.getHandler()); + broadcastDeviceListChange_sync(delegate.getHandler()); } } } @@ -4836,9 +4836,9 @@ public class AudioManager { /** * Internal method to compute and generate add/remove messages and then send to any - * registered callbacks. + * registered callbacks. Must be called synchronized on mDeviceCallbacks. */ - private void broadcastDeviceListChange(Handler handler) { + private void broadcastDeviceListChange_sync(Handler handler) { int status; // Get the new current set of ports @@ -4860,20 +4860,18 @@ public class AudioManager { AudioDeviceInfo[] removed_devices = calcListDeltas(current_ports, mPreviousPorts, GET_DEVICES_ALL); if (added_devices.length != 0 || removed_devices.length != 0) { - synchronized (mDeviceCallbacks) { - for (int i = 0; i < mDeviceCallbacks.size(); i++) { - handler = mDeviceCallbacks.valueAt(i).getHandler(); - if (handler != null) { - if (removed_devices.length != 0) { - handler.sendMessage(Message.obtain(handler, - MSG_DEVICES_DEVICES_REMOVED, - removed_devices)); - } - if (added_devices.length != 0) { - handler.sendMessage(Message.obtain(handler, - MSG_DEVICES_DEVICES_ADDED, - added_devices)); - } + for (int i = 0; i < mDeviceCallbacks.size(); i++) { + handler = mDeviceCallbacks.valueAt(i).getHandler(); + if (handler != null) { + if (removed_devices.length != 0) { + handler.sendMessage(Message.obtain(handler, + MSG_DEVICES_DEVICES_REMOVED, + removed_devices)); + } + if (added_devices.length != 0) { + handler.sendMessage(Message.obtain(handler, + MSG_DEVICES_DEVICES_ADDED, + added_devices)); } } } @@ -4889,7 +4887,9 @@ public class AudioManager { private class OnAmPortUpdateListener implements AudioManager.OnAudioPortUpdateListener { static final String TAG = "OnAmPortUpdateListener"; public void onAudioPortListUpdate(AudioPort[] portList) { - broadcastDeviceListChange(null); + synchronized (mDeviceCallbacks) { + broadcastDeviceListChange_sync(null); + } } /** @@ -4903,7 +4903,9 @@ public class AudioManager { * Callback method called when the mediaserver dies */ public void onServiceDied() { - broadcastDeviceListChange(null); + synchronized (mDeviceCallbacks) { + broadcastDeviceListChange_sync(null); + } } }