From 4ca33d655a3f7b75a39926ac61e3db4953dbd7cb Mon Sep 17 00:00:00 2001 From: Weilin Xu Date: Thu, 16 Jun 2022 19:47:17 +0000 Subject: [PATCH] Fix deadlock due to callbacks in ProgramList Copy callback member variables in ProgramList and call them after releasing the lock of ProgramList, avoiding acquiring lock in RadioAppService after already locking ProgramList, which causes deadlook. Bug: 193041795 Test: m -j Test: atest android.hardware.radio.tests.functional Test: atest com.android.server.broadcastradio.hal2 Change-Id: I196377ea030248a5d66a4db03ffb3bee4a70e633 --- .../android/hardware/radio/ProgramList.java | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/core/java/android/hardware/radio/ProgramList.java b/core/java/android/hardware/radio/ProgramList.java index 3a042a5dee4d9..e8e4fc988937a 100644 --- a/core/java/android/hardware/radio/ProgramList.java +++ b/core/java/android/hardware/radio/ProgramList.java @@ -26,7 +26,6 @@ import android.os.Parcelable; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -173,38 +172,63 @@ public final class ProgramList implements AutoCloseable { } } - void apply(@NonNull Chunk chunk) { + void apply(Chunk chunk) { + List removedList = new ArrayList<>(); + List changedList = new ArrayList<>(); + List listCallbacksCopied; + List onCompleteListenersCopied = new ArrayList<>(); synchronized (mLock) { if (mIsClosed) return; mIsComplete = false; + listCallbacksCopied = new ArrayList<>(mListCallbacks); if (chunk.isPurge()) { - new HashSet<>(mPrograms.keySet()).stream().forEach(id -> removeLocked(id)); + for (ProgramSelector.Identifier id : mPrograms.keySet()) { + removeLocked(id, removedList); + } } - chunk.getRemoved().stream().forEach(id -> removeLocked(id)); - chunk.getModified().stream().forEach(info -> putLocked(info)); + chunk.getRemoved().stream().forEach(id -> removeLocked(id, removedList)); + chunk.getModified().stream().forEach(info -> putLocked(info, changedList)); if (chunk.isComplete()) { mIsComplete = true; - mOnCompleteListeners.forEach(cb -> cb.onComplete()); + onCompleteListenersCopied = new ArrayList<>(mOnCompleteListeners); + } + } + + for (int i = 0; i < removedList.size(); i++) { + for (int cbIndex = 0; cbIndex < listCallbacksCopied.size(); cbIndex++) { + listCallbacksCopied.get(cbIndex).onItemRemoved(removedList.get(i)); + } + } + for (int i = 0; i < changedList.size(); i++) { + for (int cbIndex = 0; cbIndex < listCallbacksCopied.size(); cbIndex++) { + listCallbacksCopied.get(cbIndex).onItemChanged(changedList.get(i)); + } + } + if (chunk.isComplete()) { + for (int cbIndex = 0; cbIndex < onCompleteListenersCopied.size(); cbIndex++) { + onCompleteListenersCopied.get(cbIndex).onComplete(); } } } - private void putLocked(@NonNull RadioManager.ProgramInfo value) { + private void putLocked(RadioManager.ProgramInfo value, + List changedIdentifierList) { ProgramSelector.Identifier key = value.getSelector().getPrimaryId(); mPrograms.put(Objects.requireNonNull(key), value); ProgramSelector.Identifier sel = value.getSelector().getPrimaryId(); - mListCallbacks.forEach(cb -> cb.onItemChanged(sel)); + changedIdentifierList.add(sel); } - private void removeLocked(@NonNull ProgramSelector.Identifier key) { + private void removeLocked(ProgramSelector.Identifier key, + List removedIdentifierList) { RadioManager.ProgramInfo removed = mPrograms.remove(Objects.requireNonNull(key)); if (removed == null) return; ProgramSelector.Identifier sel = removed.getSelector().getPrimaryId(); - mListCallbacks.forEach(cb -> cb.onItemRemoved(sel)); + removedIdentifierList.add(sel); } /**