Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor locks in radio client for HIDL HAL

Browse Source 
Seperate locks were used for radio service implementation, radio
module, and tuner session in HIDL 2.0 and 1.x HAL clients to
minimize performance issue. Potential dead lock issue in
TunerSession#close was also fixed.

Bug: 258688572
Test: atest com.android.server.broadcastradio.hal2
Test: atest android.broadcastradio.cts
Change-Id: I63b1ee6b2d22c424133e7f208148e2863078cc40
This commit is contained in:
Weilin Xu
2022-11-30 23:00:38 +00:00
parent c8b8b2986c
commit 6402193c32
9 changed files with 108 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/BroadcastRadioServiceHidlTest.java
View File

@@ -59,8 +59,6 @@ public final class BroadcastRadioServiceHidlTest extends ExtendedRadioMockitoTes
new ArrayList<>(Arrays.asList("FmService", "DabService"));
private static final int[] TEST_ENABLED_TYPES = new int[]{Announcement.TYPE_TRAFFIC};
private final Object mLock = new Object();
private BroadcastRadioService mBroadcastRadioService;
private DeathRecipient mFmDeathRecipient;
@@ -200,7 +198,7 @@ public final class BroadcastRadioServiceHidlTest extends ExtendedRadioMockitoTes
mockServiceManager();
mBroadcastRadioService = new BroadcastRadioService(/* nextModuleId= */ FM_RADIO_MODULE_ID,
mLock, mServiceManagerMock);
mServiceManagerMock);
}
private void mockServiceManager() throws RemoteException {
@@ -221,9 +219,9 @@ public final class BroadcastRadioServiceHidlTest extends ExtendedRadioMockitoTes
}).thenReturn(true);
doReturn(mFmRadioModuleMock).when(() -> RadioModule.tryLoadingModule(
eq(FM_RADIO_MODULE_ID), anyString(), any(Object.class)));
eq(FM_RADIO_MODULE_ID), anyString()));
doReturn(mDabRadioModuleMock).when(() -> RadioModule.tryLoadingModule(
eq(DAB_RADIO_MODULE_ID), anyString(), any(Object.class)));
eq(DAB_RADIO_MODULE_ID), anyString()));
when(mFmRadioModuleMock.getProperties()).thenReturn(mFmModuleMock);
when(mDabRadioModuleMock.getProperties()).thenReturn(mDabModuleMock);

3
core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/RadioModuleHidlTest.java
View File

@@ -62,13 +62,12 @@ public final class RadioModuleHidlTest {
@Mock
private android.hardware.broadcastradio.V2_0.ICloseHandle mHalCloseHandleMock;
private final Object mLock = new Object();
private RadioModule mRadioModule;
private android.hardware.broadcastradio.V2_0.IAnnouncementListener mHalListener;
@Before
public void setup() throws RemoteException {
mRadioModule = new RadioModule(mBroadcastRadioMock, TEST_MODULE_PROPERTIES, mLock);
mRadioModule = new RadioModule(mBroadcastRadioMock, TEST_MODULE_PROPERTIES);
when(mBroadcastRadioMock.getImage(anyInt())).thenReturn(new ArrayList<Byte>(0));

3
core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/StartProgramListUpdatesFanoutTest.java
View File

@@ -64,7 +64,6 @@ public class StartProgramListUpdatesFanoutTest extends ExtendedRadioMockitoTestC
@Mock ITunerSession mHalTunerSessionMock;
private android.hardware.radio.ITunerCallback[] mAidlTunerCallbackMocks;
private final Object mLock = new Object();
// RadioModule under test
private RadioModule mRadioModule;
@@ -100,7 +99,7 @@ public class StartProgramListUpdatesFanoutTest extends ExtendedRadioMockitoTestC
doReturn(true).when(() -> RadioServiceUserController.isCurrentOrSystemUser());
mRadioModule = new RadioModule(mBroadcastRadioMock,
TestUtils.makeDefaultModuleProperties(), mLock);
TestUtils.makeDefaultModuleProperties());
doAnswer((Answer) invocation -> {
mHalTunerCallback = (ITunerCallback) invocation.getArguments()[0];

3
core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/TunerSessionHidlTest.java
View File

@@ -84,7 +84,6 @@ public final class TunerSessionHidlTest extends ExtendedRadioMockitoTestCase {
new RadioManager.FmBandConfig(FM_BAND_DESCRIPTOR);
private static final int UNSUPPORTED_CONFIG_FLAG = 0;
private final Object mLock = new Object();
private final ArrayMap<Integer, Boolean> mHalConfigMap = new ArrayMap<>();
private RadioModule mRadioModule;
private ITunerCallback mHalTunerCallback;
@@ -105,7 +104,7 @@ public final class TunerSessionHidlTest extends ExtendedRadioMockitoTestCase {
doReturn(true).when(() -> RadioServiceUserController.isCurrentOrSystemUser());
mRadioModule = new RadioModule(mBroadcastRadioMock,
TestUtils.makeDefaultModuleProperties(), mLock);
TestUtils.makeDefaultModuleProperties());
doAnswer(invocation -> {
mHalTunerCallback = (ITunerCallback) invocation.getArguments()[0];

19
services/core/java/com/android/server/broadcastradio/IRadioServiceHidlImpl.java
View File

@@ -27,6 +27,7 @@ import android.util.IndentingPrintWriter;
import android.util.Log;
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.server.broadcastradio.hal2.AnnouncementAggregator;
@@ -51,15 +52,17 @@ final class IRadioServiceHidlImpl extends IRadioService.Stub {
private final Object mLock = new Object();
private final BroadcastRadioService mService;
@GuardedBy("mLock")
private final List<RadioManager.ModuleProperties> mV1Modules;
IRadioServiceHidlImpl(BroadcastRadioService service) {
mService = Objects.requireNonNull(service, "broadcast radio service cannot be null");
mHal1 = new com.android.server.broadcastradio.hal1.BroadcastRadioService(mLock);
mHal1 = new com.android.server.broadcastradio.hal1.BroadcastRadioService();
mV1Modules = mHal1.loadModules();
OptionalInt max = mV1Modules.stream().mapToInt(RadioManager.ModuleProperties::getId).max();
mHal2 = new com.android.server.broadcastradio.hal2.BroadcastRadioService(
max.isPresent() ? max.getAsInt() + 1 : 0, mLock);
max.isPresent() ? max.getAsInt() + 1 : 0);
}
@VisibleForTesting
@@ -78,9 +81,11 @@ final class IRadioServiceHidlImpl extends IRadioService.Stub {
public List<RadioManager.ModuleProperties> listModules() {
mService.enforcePolicyAccess();
Collection<RadioManager.ModuleProperties> v2Modules = mHal2.listModules();
List<RadioManager.ModuleProperties> modules = new ArrayList<>(
mV1Modules.size() + v2Modules.size());
modules.addAll(mV1Modules);
List<RadioManager.ModuleProperties> modules;
synchronized (mLock) {
modules = new ArrayList<>(mV1Modules.size() + v2Modules.size());
modules.addAll(mV1Modules);
}
modules.addAll(v2Modules);
return modules;
}
@@ -131,7 +136,9 @@ final class IRadioServiceHidlImpl extends IRadioService.Stub {
radioPw.printf("HAL1: %s\n", mHal1);
radioPw.increaseIndent();
radioPw.printf("Modules of HAL1: %s\n", mV1Modules);
synchronized (mLock) {
radioPw.printf("Modules of HAL1: %s\n", mV1Modules);
}
radioPw.decreaseIndent();
radioPw.printf("HAL2:\n");

10
services/core/java/com/android/server/broadcastradio/hal1/BroadcastRadioService.java
View File

@@ -36,7 +36,7 @@ public class BroadcastRadioService {
*/
private final long mNativeContext = nativeInit();
private final Object mLock;
private final Object mLock = new Object();
@Override
protected void finalize() throws Throwable {
@@ -50,14 +50,6 @@ public class BroadcastRadioService {
private native Tuner nativeOpenTuner(long nativeContext, int moduleId,
RadioManager.BandConfig config, boolean withAudio, ITunerCallback callback);
/**
* Constructor. should pass
* {@code com.android.server.broadcastradio.BroadcastRadioService#mLock} for lock.
*/
public BroadcastRadioService(Object lock) {
mLock = lock;
}
public @NonNull List<RadioManager.ModuleProperties> loadModules() {
synchronized (mLock) {
return Objects.requireNonNull(nativeLoadModules(mNativeContext));

19
services/core/java/com/android/server/broadcastradio/hal2/BroadcastRadioService.java
View File

@@ -43,13 +43,16 @@ import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
public class BroadcastRadioService {
/**
* Broadcast radio service using BroadcastRadio HIDL 2.0 HAL
*/
public final class BroadcastRadioService {
private static final String TAG = "BcRadio2Srv";
private final Object mLock;
private final Object mLock = new Object();
@GuardedBy("mLock")
private int mNextModuleId = 0;
private int mNextModuleId;
@GuardedBy("mLock")
private final Map<String, Integer> mServiceNameToModuleIdMap = new HashMap<>();
@@ -72,7 +75,7 @@ public class BroadcastRadioService {
moduleId = mNextModuleId;
}
RadioModule module = RadioModule.tryLoadingModule(moduleId, serviceName, mLock);
RadioModule module = RadioModule.tryLoadingModule(moduleId, serviceName);
if (module == null) {
return;
}
@@ -120,9 +123,8 @@ public class BroadcastRadioService {
}
};
public BroadcastRadioService(int nextModuleId, Object lock) {
public BroadcastRadioService(int nextModuleId) {
mNextModuleId = nextModuleId;
mLock = lock;
try {
IServiceManager manager = IServiceManager.getService();
if (manager == null) {
@@ -136,9 +138,8 @@ public class BroadcastRadioService {
}
@VisibleForTesting
BroadcastRadioService(int nextModuleId, Object lock, IServiceManager manager) {
BroadcastRadioService(int nextModuleId, IServiceManager manager) {
mNextModuleId = nextModuleId;
mLock = lock;
Objects.requireNonNull(manager, "Service manager cannot be null");
try {
manager.registerForNotifications(IBroadcastRadio.kInterfaceName, "", mServiceListener);
@@ -180,7 +181,7 @@ public class BroadcastRadioService {
throw new IllegalArgumentException("Non-audio sessions not supported with HAL 2.0");
}
RadioModule module = null;
RadioModule module;
synchronized (mLock) {
module = mModules.get(moduleId);
if (module == null) {

101
services/core/java/com/android/server/broadcastradio/hal2/RadioModule.java
View File

@@ -60,7 +60,7 @@ final class RadioModule {
@NonNull private final IBroadcastRadio mService;
@NonNull private final RadioManager.ModuleProperties mProperties;
private final Object mLock;
private final Object mLock = new Object();
@NonNull private final Handler mHandler;
@NonNull private final RadioEventLogger mEventLogger;
@@ -75,7 +75,7 @@ final class RadioModule {
private RadioManager.ProgramInfo mCurrentProgramInfo = null;
@GuardedBy("mLock")
private final ProgramInfoCache mProgramInfoCache = new ProgramInfoCache(null);
private final ProgramInfoCache mProgramInfoCache = new ProgramInfoCache(/* filter= */ null);
@GuardedBy("mLock")
private android.hardware.radio.ProgramList.Filter mUnionOfAidlProgramFilters = null;
@@ -84,47 +84,59 @@ final class RadioModule {
private final ITunerCallback mHalTunerCallback = new ITunerCallback.Stub() {
@Override
public void onTuneFailed(int result, ProgramSelector programSelector) {
lockAndFireLater(() -> {
fireLater(() -> {
android.hardware.radio.ProgramSelector csel =
Convert.programSelectorFromHal(programSelector);
fanoutAidlCallbackLocked(cb -> cb.onTuneFailed(result, csel));
synchronized (mLock) {
fanoutAidlCallbackLocked(cb -> cb.onTuneFailed(result, csel));
}
});
}
@Override
public void onCurrentProgramInfoChanged(ProgramInfo halProgramInfo) {
lockAndFireLater(() -> {
mCurrentProgramInfo = Convert.programInfoFromHal(halProgramInfo);
fanoutAidlCallbackLocked(cb -> cb.onCurrentProgramInfoChanged(mCurrentProgramInfo));
fireLater(() -> {
synchronized (mLock) {
mCurrentProgramInfo = Convert.programInfoFromHal(halProgramInfo);
RadioManager.ProgramInfo currentProgramInfo = mCurrentProgramInfo;
fanoutAidlCallbackLocked(cb -> cb.onCurrentProgramInfoChanged(
currentProgramInfo));
}
});
}
@Override
public void onProgramListUpdated(ProgramListChunk programListChunk) {
lockAndFireLater(() -> {
fireLater(() -> {
android.hardware.radio.ProgramList.Chunk chunk =
Convert.programListChunkFromHal(programListChunk);
mProgramInfoCache.filterAndApplyChunk(chunk);
synchronized (mLock) {
mProgramInfoCache.filterAndApplyChunk(chunk);
for (TunerSession tunerSession : mAidlTunerSessions) {
tunerSession.onMergedProgramListUpdateFromHal(chunk);
for (TunerSession tunerSession : mAidlTunerSessions) {
tunerSession.onMergedProgramListUpdateFromHal(chunk);
}
}
});
}
@Override
public void onAntennaStateChange(boolean connected) {
lockAndFireLater(() -> {
mAntennaConnected = connected;
fanoutAidlCallbackLocked(cb -> cb.onAntennaState(connected));
fireLater(() -> {
synchronized (mLock) {
mAntennaConnected = connected;
fanoutAidlCallbackLocked(cb -> cb.onAntennaState(connected));
}
});
}
@Override
public void onParametersUpdated(ArrayList<VendorKeyValue> parameters) {
lockAndFireLater(() -> {
fireLater(() -> {
Map<String, String> cparam = Convert.vendorInfoFromHal(parameters);
fanoutAidlCallbackLocked(cb -> cb.onParametersUpdated(cparam));
synchronized (mLock) {
fanoutAidlCallbackLocked(cb -> cb.onParametersUpdated(cparam));
}
});
}
};
@@ -135,17 +147,15 @@ final class RadioModule {
@VisibleForTesting
RadioModule(@NonNull IBroadcastRadio service,
@NonNull RadioManager.ModuleProperties properties, @NonNull Object lock) {
@NonNull RadioManager.ModuleProperties properties) {
mProperties = Objects.requireNonNull(properties);
mService = Objects.requireNonNull(service);
mLock = Objects.requireNonNull(lock);
mHandler = new Handler(Looper.getMainLooper());
mEventLogger = new RadioEventLogger(TAG, RADIO_EVENT_LOGGER_QUEUE_SIZE);
}
@Nullable
static RadioModule tryLoadingModule(int idx, @NonNull String fqName,
Object lock) {
static RadioModule tryLoadingModule(int idx, @NonNull String fqName) {
try {
Slog.i(TAG, "Try loading module for idx " + idx + ", fqName " + fqName);
IBroadcastRadio service = IBroadcastRadio.getService(fqName);
@@ -167,7 +177,7 @@ final class RadioModule {
RadioManager.ModuleProperties prop = Convert.propertiesFromHal(idx, fqName,
service.getProperties(), amfmConfig.value, dabConfig.value);
return new RadioModule(service, prop, lock);
return new RadioModule(service, prop);
} catch (RemoteException ex) {
Slog.e(TAG, "Failed to load module " + fqName, ex);
return null;
@@ -196,8 +206,7 @@ final class RadioModule {
});
mHalTunerSession = Objects.requireNonNull(hwSession.value);
}
TunerSession tunerSession = new TunerSession(this, mHalTunerSession, userCb,
mLock);
TunerSession tunerSession = new TunerSession(this, mHalTunerSession, userCb);
mAidlTunerSessions.add(tunerSession);
// Propagate state to new client. Note: These callbacks are invoked while holding mLock
@@ -229,6 +238,7 @@ final class RadioModule {
}
}
@GuardedBy("mLock")
@Nullable
private android.hardware.radio.ProgramList.Filter
buildUnionOfTunerSessionFiltersLocked() {
@@ -281,6 +291,7 @@ final class RadioModule {
}
}
@GuardedBy("mLock")
private void onTunerSessionProgramListFilterChangedLocked(@Nullable TunerSession session) {
android.hardware.radio.ProgramList.Filter newFilter =
buildUnionOfTunerSessionFiltersLocked();
@@ -325,6 +336,7 @@ final class RadioModule {
}
}
@GuardedBy("mLock")
private void onTunerSessionsClosedLocked(TunerSession... tunerSessions) {
for (TunerSession tunerSession : tunerSessions) {
mAidlTunerSessions.remove(tunerSession);
@@ -342,12 +354,8 @@ final class RadioModule {
}
// add to mHandler queue, but ensure the runnable holds mLock when it gets executed
private void lockAndFireLater(Runnable r) {
mHandler.post(() -> {
synchronized (mLock) {
r.run();
}
});
private void fireLater(Runnable r) {
mHandler.post(() -> r.run());
}
interface AidlCallbackRunnable {
@@ -356,9 +364,14 @@ final class RadioModule {
// Invokes runnable with each TunerSession currently open.
void fanoutAidlCallback(AidlCallbackRunnable runnable) {
lockAndFireLater(() -> fanoutAidlCallbackLocked(runnable));
fireLater(() -> {
synchronized (mLock) {
fanoutAidlCallbackLocked(runnable);
}
});
}
@GuardedBy("mLock")
private void fanoutAidlCallbackLocked(AidlCallbackRunnable runnable) {
List<TunerSession> deadSessions = null;
for (TunerSession tunerSession : mAidlTunerSessions) {
@@ -399,12 +412,10 @@ final class RadioModule {
}
};
synchronized (mLock) {
mService.registerAnnouncementListener(enabledList, hwListener, (result, closeHnd) -> {
halResult.value = result;
hwCloseHandle.value = closeHnd;
});
}
mService.registerAnnouncementListener(enabledList, hwListener, (result, closeHandle) -> {
halResult.value = result;
hwCloseHandle.value = closeHandle;
});
Convert.throwOnError("addAnnouncementListener", halResult.value);
return new android.hardware.radio.ICloseHandle.Stub() {
@@ -424,12 +435,10 @@ final class RadioModule {
if (id == 0) throw new IllegalArgumentException("Image ID is missing");
byte[] rawImage;
synchronized (mLock) {
List<Byte> rawList = Utils.maybeRethrow(() -> mService.getImage(id));
rawImage = new byte[rawList.size()];
for (int i = 0; i < rawList.size(); i++) {
rawImage[i] = rawList.get(i);
}
List<Byte> rawList = Utils.maybeRethrow(() -> mService.getImage(id));
rawImage = new byte[rawList.size()];
for (int i = 0; i < rawList.size(); i++) {
rawImage[i] = rawList.get(i);
}
if (rawImage == null || rawImage.length == 0) return null;
@@ -440,17 +449,17 @@ final class RadioModule {
void dumpInfo(IndentingPrintWriter pw) {
pw.printf("RadioModule\n");
pw.increaseIndent();
pw.printf("BroadcastRadioService: %s\n", mService);
pw.printf("Properties: %s\n", mProperties);
synchronized (mLock) {
pw.printf("BroadcastRadioService: %s\n", mService);
pw.printf("Properties: %s\n", mProperties);
pw.printf("HIDL2.0 HAL TunerSession: %s\n", mHalTunerSession);
pw.printf("HIDL 2.0 HAL TunerSession: %s\n", mHalTunerSession);
pw.printf("Is antenna connected? ");
if (mAntennaConnected == null) {
pw.printf("null\n");
} else {
pw.printf("%s\n", mAntennaConnected ? "Yes" : "No");
}
pw.printf("current ProgramInfo: %s\n", mCurrentProgramInfo);
pw.printf("Current ProgramInfo: %s\n", mCurrentProgramInfo);
pw.printf("ProgramInfoCache: %s\n", mProgramInfoCache);
pw.printf("Union of AIDL ProgramFilters: %s\n", mUnionOfAidlProgramFilters);
pw.printf("AIDL TunerSessions:\n");

40
services/core/java/com/android/server/broadcastradio/hal2/TunerSession.java
View File

@@ -33,6 +33,7 @@ import android.util.MutableBoolean;
import android.util.MutableInt;
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
import com.android.server.broadcastradio.RadioServiceUserController;
import com.android.server.utils.Slogf;
@@ -46,26 +47,28 @@ class TunerSession extends ITuner.Stub {
private static final String kAudioDeviceName = "Radio tuner source";
private static final int TUNER_EVENT_LOGGER_QUEUE_SIZE = 25;
private final Object mLock;
private final Object mLock = new Object();
@NonNull private final RadioEventLogger mEventLogger;
private final RadioModule mModule;
private final ITunerSession mHwSession;
final android.hardware.radio.ITunerCallback mCallback;
@GuardedBy("mLock")
private boolean mIsClosed = false;
@GuardedBy("mLock")
private boolean mIsMuted = false;
@GuardedBy("mLock")
private ProgramInfoCache mProgramInfoCache = null;
// necessary only for older APIs compatibility
private RadioManager.BandConfig mDummyConfig = null;
TunerSession(@NonNull RadioModule module, @NonNull ITunerSession hwSession,
@NonNull android.hardware.radio.ITunerCallback callback,
@NonNull Object lock) {
@NonNull android.hardware.radio.ITunerCallback callback) {
mModule = Objects.requireNonNull(module);
mHwSession = Objects.requireNonNull(hwSession);
mCallback = Objects.requireNonNull(callback);
mLock = Objects.requireNonNull(lock);
mEventLogger = new RadioEventLogger(TAG, TUNER_EVENT_LOGGER_QUEUE_SIZE);
}
@@ -86,23 +89,26 @@ class TunerSession extends ITuner.Stub {
mEventLogger.logRadioEvent("Close on error %d", error);
synchronized (mLock) {
if (mIsClosed) return;
if (error != null) {
try {
mCallback.onError(error);
} catch (RemoteException ex) {
Slog.w(TAG, "mCallback.onError() failed: ", ex);
}
}
mIsClosed = true;
mModule.onTunerSessionClosed(this);
}
if (error != null) {
try {
mCallback.onError(error);
} catch (RemoteException ex) {
Slog.w(TAG, "mCallback.onError() failed: ", ex);
}
}
mModule.onTunerSessionClosed(this);
}
@Override
public boolean isClosed() {
return mIsClosed;
synchronized (mLock) {
return mIsClosed;
}
}
@GuardedBy("mLock")
private void checkNotClosedLocked() {
if (mIsClosed) {
throw new IllegalStateException("Tuner is closed, no further operations are allowed");
@@ -118,9 +124,9 @@ class TunerSession extends ITuner.Stub {
synchronized (mLock) {
checkNotClosedLocked();
mDummyConfig = Objects.requireNonNull(config);
Slog.i(TAG, "Ignoring setConfiguration - not applicable for broadcastradio HAL 2.0");
mModule.fanoutAidlCallback(cb -> cb.onConfigurationChanged(config));
}
Slog.i(TAG, "Ignoring setConfiguration - not applicable for broadcastradio HAL 2.0");
mModule.fanoutAidlCallback(cb -> cb.onConfigurationChanged(config));
}
@Override
@@ -137,8 +143,8 @@ class TunerSession extends ITuner.Stub {
checkNotClosedLocked();
if (mIsMuted == mute) return;
mIsMuted = mute;
Slog.w(TAG, "Mute via RadioService is not implemented - please handle it via app");
}
Slog.w(TAG, "Mute via RadioService is not implemented - please handle it via app");
}
@Override
@@ -383,8 +389,8 @@ class TunerSession extends ITuner.Stub {
void dumpInfo(IndentingPrintWriter pw) {
pw.printf("TunerSession\n");
pw.increaseIndent();
pw.printf("HIDL HAL Session: %s\n", mHwSession);
synchronized (mLock) {
pw.printf("HIDL HAL Session: %s\n", mHwSession);
pw.printf("Is session closed? %s\n", mIsClosed ? "Yes" : "No");
pw.printf("Is muted? %s\n", mIsMuted ? "Yes" : "No");
pw.printf("ProgramInfoCache: %s\n", mProgramInfoCache);