Refactor locks in radio client for AIDL HAL
Seperate locks were used for radio service implementation, radio module, and tuner session for broadcast radio AIDL HAL client to minimize performance issue. Potential dead lock issue in TunerSession#close was also fixed. Bug: 258688572 Test: atest com.android.server.broadcastradio.aidl Test: atest android.broadcastradio.cts Change-Id: I8de3afa2935e077488e7575bb9b64495c2113292
This commit is contained in:
@@ -210,9 +210,9 @@ public final class BroadcastRadioServiceImplTest extends ExtendedRadioMockitoTes
|
||||
any(IServiceCallback.class)));
|
||||
|
||||
doReturn(mFmRadioModuleMock).when(() -> RadioModule.tryLoadingModule(
|
||||
eq(FM_RADIO_MODULE_ID), anyString(), any(IBinder.class), any(Object.class)));
|
||||
eq(FM_RADIO_MODULE_ID), anyString(), any(IBinder.class)));
|
||||
doReturn(mDabRadioModuleMock).when(() -> RadioModule.tryLoadingModule(
|
||||
eq(DAB_RADIO_MODULE_ID), anyString(), any(IBinder.class), any(Object.class)));
|
||||
eq(DAB_RADIO_MODULE_ID), anyString(), any(IBinder.class)));
|
||||
|
||||
when(mFmRadioModuleMock.getProperties()).thenReturn(mFmModuleMock);
|
||||
when(mDabRadioModuleMock.getProperties()).thenReturn(mDabModuleMock);
|
||||
|
||||
@@ -58,14 +58,13 @@ public final class RadioModuleTest {
|
||||
@Mock
|
||||
private android.hardware.broadcastradio.ICloseHandle mHalCloseHandleMock;
|
||||
|
||||
private final Object mLock = new Object();
|
||||
// RadioModule under test
|
||||
private RadioModule mRadioModule;
|
||||
private android.hardware.broadcastradio.IAnnouncementListener mHalListener;
|
||||
|
||||
@Before
|
||||
public void setup() throws RemoteException {
|
||||
mRadioModule = new RadioModule(mBroadcastRadioMock, TEST_MODULE_PROPERTIES, mLock);
|
||||
mRadioModule = new RadioModule(mBroadcastRadioMock, TEST_MODULE_PROPERTIES);
|
||||
|
||||
// TODO(b/241118988): test non-null image for getImage method
|
||||
when(mBroadcastRadioMock.getImage(anyInt())).thenReturn(null);
|
||||
|
||||
@@ -83,7 +83,6 @@ public final class TunerSessionTest extends ExtendedRadioMockitoTestCase {
|
||||
@Mock private IBroadcastRadio mBroadcastRadioMock;
|
||||
private android.hardware.radio.ITunerCallback[] mAidlTunerCallbackMocks;
|
||||
|
||||
private final Object mLock = new Object();
|
||||
// RadioModule under test
|
||||
private RadioModule mRadioModule;
|
||||
|
||||
@@ -104,7 +103,7 @@ public final class TunerSessionTest extends ExtendedRadioMockitoTestCase {
|
||||
doReturn(true).when(() -> RadioServiceUserController.isCurrentOrSystemUser());
|
||||
|
||||
mRadioModule = new RadioModule(mBroadcastRadioMock,
|
||||
AidlTestUtils.makeDefaultModuleProperties(), mLock);
|
||||
AidlTestUtils.makeDefaultModuleProperties());
|
||||
|
||||
doAnswer(invocation -> {
|
||||
mHalTunerCallback = (ITunerCallback) invocation.getArguments()[0];
|
||||
|
||||
@@ -77,7 +77,7 @@ public final class BroadcastRadioServiceImpl {
|
||||
}
|
||||
|
||||
RadioModule radioModule =
|
||||
RadioModule.tryLoadingModule(moduleId, name, newBinder, mLock);
|
||||
RadioModule.tryLoadingModule(moduleId, name, newBinder);
|
||||
if (radioModule == null) {
|
||||
Slogf.w(TAG, "No module %s with id %d (HAL AIDL)", name, moduleId);
|
||||
return;
|
||||
|
||||
@@ -55,7 +55,7 @@ final class RadioModule {
|
||||
|
||||
private final IBroadcastRadio mService;
|
||||
|
||||
private final Object mLock;
|
||||
private final Object mLock = new Object();
|
||||
private final Handler mHandler;
|
||||
private final RadioLogger mLogger;
|
||||
private final RadioManager.ModuleProperties mProperties;
|
||||
@@ -165,18 +165,15 @@ final class RadioModule {
|
||||
};
|
||||
|
||||
@VisibleForTesting
|
||||
RadioModule(IBroadcastRadio service,
|
||||
RadioManager.ModuleProperties properties, Object lock) {
|
||||
RadioModule(IBroadcastRadio service, RadioManager.ModuleProperties properties) {
|
||||
mProperties = Objects.requireNonNull(properties, "properties cannot be null");
|
||||
mService = Objects.requireNonNull(service, "service cannot be null");
|
||||
mLock = Objects.requireNonNull(lock, "lock cannot be null");
|
||||
mHandler = new Handler(Looper.getMainLooper());
|
||||
mLogger = new RadioLogger(TAG, RADIO_EVENT_LOGGER_QUEUE_SIZE);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
static RadioModule tryLoadingModule(int moduleId, String moduleName,
|
||||
IBinder serviceBinder, Object lock) {
|
||||
static RadioModule tryLoadingModule(int moduleId, String moduleName, IBinder serviceBinder) {
|
||||
try {
|
||||
Slogf.i(TAG, "Try loading module for module id = %d, module name = %s",
|
||||
moduleId, moduleName);
|
||||
@@ -206,7 +203,7 @@ final class RadioModule {
|
||||
RadioManager.ModuleProperties prop = ConversionUtils.propertiesFromHalProperties(
|
||||
moduleId, moduleName, service.getProperties(), amfmConfig, dabConfig);
|
||||
|
||||
return new RadioModule(service, prop, lock);
|
||||
return new RadioModule(service, prop);
|
||||
} catch (RemoteException ex) {
|
||||
Slogf.e(TAG, ex, "Failed to load module %s", moduleName);
|
||||
return null;
|
||||
@@ -222,9 +219,7 @@ final class RadioModule {
|
||||
}
|
||||
|
||||
void setInternalHalCallback() throws RemoteException {
|
||||
synchronized (mLock) {
|
||||
mService.setTunerCallback(mHalTunerCallback);
|
||||
}
|
||||
mService.setTunerCallback(mHalTunerCallback);
|
||||
}
|
||||
|
||||
TunerSession openSession(android.hardware.radio.ITunerCallback userCb)
|
||||
@@ -234,7 +229,7 @@ final class RadioModule {
|
||||
Boolean antennaConnected;
|
||||
RadioManager.ProgramInfo currentProgramInfo;
|
||||
synchronized (mLock) {
|
||||
tunerSession = new TunerSession(this, mService, userCb, mLock);
|
||||
tunerSession = new TunerSession(this, mService, userCb);
|
||||
mAidlTunerSessions.add(tunerSession);
|
||||
antennaConnected = mAntennaConnected;
|
||||
currentProgramInfo = mCurrentProgramInfo;
|
||||
@@ -356,14 +351,14 @@ final class RadioModule {
|
||||
// Otherwise, update the HAL's filter, and AIDL clients will be updated when
|
||||
// mHalTunerCallback.onProgramListUpdated() is called.
|
||||
mUnionOfAidlProgramFilters = newFilter;
|
||||
try {
|
||||
mService.startProgramListUpdates(
|
||||
ConversionUtils.filterToHalProgramFilter(newFilter));
|
||||
} catch (RuntimeException ex) {
|
||||
throw ConversionUtils.throwOnError(ex, /* action= */ "Start Program ListUpdates");
|
||||
} catch (RemoteException ex) {
|
||||
Slogf.e(TAG, ex, "mHalTunerSession.startProgramListUpdates() failed");
|
||||
}
|
||||
}
|
||||
try {
|
||||
mService.startProgramListUpdates(
|
||||
ConversionUtils.filterToHalProgramFilter(newFilter));
|
||||
} catch (RuntimeException ex) {
|
||||
throw ConversionUtils.throwOnError(ex, /* action= */ "Start Program ListUpdates");
|
||||
} catch (RemoteException ex) {
|
||||
Slogf.e(TAG, ex, "mHalTunerSession.startProgramListUpdates() failed");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -453,12 +448,10 @@ final class RadioModule {
|
||||
}
|
||||
};
|
||||
|
||||
synchronized (mLock) {
|
||||
try {
|
||||
hwCloseHandle[0] = mService.registerAnnouncementListener(hwListener, enabledList);
|
||||
} catch (RuntimeException ex) {
|
||||
throw ConversionUtils.throwOnError(ex, /* action= */ "AnnouncementListener");
|
||||
}
|
||||
try {
|
||||
hwCloseHandle[0] = mService.registerAnnouncementListener(hwListener, enabledList);
|
||||
} catch (RuntimeException ex) {
|
||||
throw ConversionUtils.throwOnError(ex, /* action= */ "AnnouncementListener");
|
||||
}
|
||||
|
||||
return new android.hardware.radio.ICloseHandle.Stub() {
|
||||
@@ -478,12 +471,10 @@ final class RadioModule {
|
||||
if (id == 0) throw new IllegalArgumentException("Image ID is missing");
|
||||
|
||||
byte[] rawImage;
|
||||
synchronized (mLock) {
|
||||
try {
|
||||
rawImage = mService.getImage(id);
|
||||
} catch (RemoteException ex) {
|
||||
throw ex.rethrowFromSystemServer();
|
||||
}
|
||||
try {
|
||||
rawImage = mService.getImage(id);
|
||||
} catch (RemoteException ex) {
|
||||
throw ex.rethrowFromSystemServer();
|
||||
}
|
||||
|
||||
if (rawImage == null || rawImage.length == 0) return null;
|
||||
|
||||
@@ -42,7 +42,7 @@ final class TunerSession extends ITuner.Stub {
|
||||
private static final String TAG = "BcRadioAidlSrv.session";
|
||||
private static final int TUNER_EVENT_LOGGER_QUEUE_SIZE = 25;
|
||||
|
||||
private final Object mLock;
|
||||
private final Object mLock = new Object();
|
||||
|
||||
private final RadioLogger mLogger;
|
||||
private final RadioModule mModule;
|
||||
@@ -61,12 +61,10 @@ final class TunerSession extends ITuner.Stub {
|
||||
private RadioManager.BandConfig mPlaceHolderConfig;
|
||||
|
||||
TunerSession(RadioModule radioModule, IBroadcastRadio service,
|
||||
android.hardware.radio.ITunerCallback callback,
|
||||
Object lock) {
|
||||
android.hardware.radio.ITunerCallback callback) {
|
||||
mModule = Objects.requireNonNull(radioModule, "radioModule cannot be null");
|
||||
mService = Objects.requireNonNull(service, "service cannot be null");
|
||||
mCallback = Objects.requireNonNull(callback, "callback cannot be null");
|
||||
mLock = Objects.requireNonNull(lock, "lock cannot be null");
|
||||
mLogger = new RadioLogger(TAG, TUNER_EVENT_LOGGER_QUEUE_SIZE);
|
||||
}
|
||||
|
||||
@@ -91,17 +89,19 @@ final class TunerSession extends ITuner.Stub {
|
||||
mLogger.logRadioEvent("Close tuner session on error %d", error);
|
||||
}
|
||||
synchronized (mLock) {
|
||||
if (mIsClosed) return;
|
||||
if (error != null) {
|
||||
try {
|
||||
mCallback.onError(error);
|
||||
} catch (RemoteException ex) {
|
||||
Slogf.w(TAG, ex, "mCallback.onError(%s) failed", error);
|
||||
}
|
||||
if (mIsClosed) {
|
||||
return;
|
||||
}
|
||||
mIsClosed = true;
|
||||
mModule.onTunerSessionClosed(this);
|
||||
}
|
||||
if (error != null) {
|
||||
try {
|
||||
mCallback.onError(error);
|
||||
} catch (RemoteException ex) {
|
||||
Slogf.w(TAG, ex, "mCallback.onError(%s) failed", error);
|
||||
}
|
||||
}
|
||||
mModule.onTunerSessionClosed(this);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
Reference in New Issue
Block a user