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

unite all locks under BroadcastRadio

Browse Source 
- Each layer having separate a lock and making calls to other layers
  while holding lock leads into deadlock.
- Change all of them to use a single lock from the service to avoid
  deadlock.
- Performance impact should be minimal as there are not that many
  players for radio (one app, HAL, and system server) and the amount
  of calls are not that frequent.

Bug: 194818704
Test: atest BroadcastRadioTests
Change-Id: I5f51c0e0a04ae10428f721be508d5579dc1dd837
This commit is contained in:
Keun young Park
2021-07-27 16:36:23 -07:00
parent 8a67185b09
commit 2cdb566acb
7 changed files with 36 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -65,6 +65,7 @@ public class StartProgramListUpdatesFanoutTest {
@Mock ITunerSession mHalTunerSessionMock;
private android.hardware.radio.ITunerCallback[] mAidlTunerCallbackMocks;
private final Object mLock = new Object();
// RadioModule under test
private RadioModule mRadioModule;
@@ -96,7 +97,7 @@ public class StartProgramListUpdatesFanoutTest {
mRadioModule = new RadioModule(mBroadcastRadioMock, new RadioManager.ModuleProperties(0, "",
0, "", "", "", "", 0, 0, false, false, null, false, new int[] {}, new int[] {},
null, null));
null, null), mLock);
doAnswer((Answer) invocation -> {
mHalTunerCallback = (ITunerCallback) invocation.getArguments()[0];

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

@@ -52,11 +52,11 @@ public class BroadcastRadioService extends SystemService {
public BroadcastRadioService(Context context) {
super(context);
mHal1 = new com.android.server.broadcastradio.hal1.BroadcastRadioService();
mHal1 = new com.android.server.broadcastradio.hal1.BroadcastRadioService(mLock);
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);
max.isPresent() ? max.getAsInt() + 1 : 0, mLock);
}
@Override
@@ -111,7 +111,7 @@ public class BroadcastRadioService extends SystemService {
synchronized (mLock) {
if (!mHal2.hasAnyModules()) {
Slog.i(TAG, "There are no HAL 2.x modules registered");
return new AnnouncementAggregator(listener);
return new AnnouncementAggregator(listener, mLock);
}
return mHal2.addAnnouncementListener(enabledTypes, listener);

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

@@ -17,16 +17,9 @@
package com.android.server.broadcastradio.hal1;
import android.annotation.NonNull;
import android.Manifest;
import android.content.Context;
import android.content.pm.PackageManager;
import android.hardware.radio.IRadioService;
import android.hardware.radio.ITuner;
import android.hardware.radio.ITunerCallback;
import android.hardware.radio.RadioManager;
import android.os.ParcelableException;
import com.android.server.SystemService;
import java.util.List;
import java.util.Objects;
@@ -37,7 +30,7 @@ public class BroadcastRadioService {
*/
private final long mNativeContext = nativeInit();
private final Object mLock = new Object();
private final Object mLock;
@Override
protected void finalize() throws Throwable {
@@ -51,6 +44,14 @@ 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(@NonNull Object lock) {
mLock = lock;
}
public @NonNull List<RadioManager.ModuleProperties> loadModules() {
synchronized (mLock) {
return Objects.requireNonNull(nativeLoadModules(mNativeContext));

5
services/core/java/com/android/server/broadcastradio/hal2/AnnouncementAggregator.java
View File

@@ -35,7 +35,7 @@ import java.util.Objects;
public class AnnouncementAggregator extends ICloseHandle.Stub {
private static final String TAG = "BcRadio2Srv.AnnAggr";
private final Object mLock = new Object();
private final Object mLock;
@NonNull private final IAnnouncementListener mListener;
private final IBinder.DeathRecipient mDeathRecipient = new DeathRecipient();
@@ -45,8 +45,9 @@ public class AnnouncementAggregator extends ICloseHandle.Stub {
@GuardedBy("mLock")
private boolean mIsClosed = false;
public AnnouncementAggregator(@NonNull IAnnouncementListener listener) {
public AnnouncementAggregator(@NonNull IAnnouncementListener listener, @NonNull Object lock) {
mListener = Objects.requireNonNull(listener);
mLock = Objects.requireNonNull(lock);
try {
listener.asBinder().linkToDeath(mDeathRecipient, 0);
} catch (RemoteException ex) {

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

@@ -42,7 +42,7 @@ import java.util.stream.Collectors;
public class BroadcastRadioService {
private static final String TAG = "BcRadio2Srv";
private final Object mLock = new Object();
private final Object mLock;
@GuardedBy("mLock")
private int mNextModuleId = 0;
@@ -68,7 +68,7 @@ public class BroadcastRadioService {
moduleId = mNextModuleId;
}
RadioModule module = RadioModule.tryLoadingModule(moduleId, serviceName);
RadioModule module = RadioModule.tryLoadingModule(moduleId, serviceName, mLock);
if (module == null) {
return;
}
@@ -116,8 +116,9 @@ public class BroadcastRadioService {
}
};
public BroadcastRadioService(int nextModuleId) {
public BroadcastRadioService(int nextModuleId, Object lock) {
mNextModuleId = nextModuleId;
mLock = lock;
try {
IServiceManager manager = IServiceManager.getService();
if (manager == null) {
@@ -174,7 +175,7 @@ public class BroadcastRadioService {
public ICloseHandle addAnnouncementListener(@NonNull int[] enabledTypes,
@NonNull IAnnouncementListener listener) {
AnnouncementAggregator aggregator = new AnnouncementAggregator(listener);
AnnouncementAggregator aggregator = new AnnouncementAggregator(listener, mLock);
boolean anySupported = false;
synchronized (mLock) {
for (RadioModule module : mModules.values()) {

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

@@ -58,7 +58,7 @@ class RadioModule {
@NonNull private final IBroadcastRadio mService;
@NonNull public final RadioManager.ModuleProperties mProperties;
private final Object mLock = new Object();
private final Object mLock;
@NonNull private final Handler mHandler;
@GuardedBy("mLock")
@@ -132,13 +132,15 @@ class RadioModule {
@VisibleForTesting
RadioModule(@NonNull IBroadcastRadio service,
@NonNull RadioManager.ModuleProperties properties) {
@NonNull RadioManager.ModuleProperties properties, @NonNull Object lock) {
mProperties = Objects.requireNonNull(properties);
mService = Objects.requireNonNull(service);
mLock = Objects.requireNonNull(lock);
mHandler = new Handler(Looper.getMainLooper());
}
public static @Nullable RadioModule tryLoadingModule(int idx, @NonNull String fqName) {
public static @Nullable RadioModule tryLoadingModule(int idx, @NonNull String fqName,
Object lock) {
try {
IBroadcastRadio service = IBroadcastRadio.getService(fqName);
if (service == null) return null;
@@ -156,7 +158,7 @@ class RadioModule {
RadioManager.ModuleProperties prop = Convert.propertiesFromHal(idx, fqName,
service.getProperties(), amfmConfig.value, dabConfig.value);
return new RadioModule(service, prop);
return new RadioModule(service, prop, lock);
} catch (RemoteException ex) {
Slog.e(TAG, "failed to load module " + fqName, ex);
return null;
@@ -178,7 +180,8 @@ class RadioModule {
});
mHalTunerSession = Objects.requireNonNull(hwSession.value);
}
TunerSession tunerSession = new TunerSession(this, mHalTunerSession, userCb);
TunerSession tunerSession = new TunerSession(this, mHalTunerSession, userCb,
mLock);
mAidlTunerSessions.add(tunerSession);
// Propagate state to new client. Note: These callbacks are invoked while holding mLock
@@ -377,7 +380,7 @@ class RadioModule {
}
};
synchronized (mService) {
synchronized (mLock) {
mService.registerAnnouncementListener(enabledList, hwListener, (result, closeHnd) -> {
halResult.value = result;
hwCloseHandle.value = closeHnd;
@@ -401,7 +404,7 @@ class RadioModule {
if (id == 0) throw new IllegalArgumentException("Image ID is missing");
byte[] rawImage;
synchronized (mService) {
synchronized (mLock) {
List<Byte> rawList = Utils.maybeRethrow(() -> mService.getImage(id));
rawImage = new byte[rawList.size()];
for (int i = 0; i < rawList.size(); i++) {

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

@@ -40,7 +40,7 @@ class TunerSession extends ITuner.Stub {
private static final String TAG = "BcRadio2Srv.session";
private static final String kAudioDeviceName = "Radio tuner source";
private final Object mLock = new Object();
private final Object mLock;
private final RadioModule mModule;
private final ITunerSession mHwSession;
@@ -53,10 +53,12 @@ class TunerSession extends ITuner.Stub {
private RadioManager.BandConfig mDummyConfig = null;
TunerSession(@NonNull RadioModule module, @NonNull ITunerSession hwSession,
@NonNull android.hardware.radio.ITunerCallback callback) {
@NonNull android.hardware.radio.ITunerCallback callback,
@NonNull Object lock) {
mModule = Objects.requireNonNull(module);
mHwSession = Objects.requireNonNull(hwSession);
mCallback = Objects.requireNonNull(callback);
mLock = Objects.requireNonNull(lock);
}
@Override