Refactor locks in radio client for HIDL HAL

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

9 files changed, 108 insertions, 98 deletions

diff --git a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/BroadcastRadioServiceHidlTest.java b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/BroadcastRadioServiceHidlTest.java
index 0b7bbeaab28e..c9224bfbe1c7 100644
--- a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/BroadcastRadioServiceHidlTest.java
+++ b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/BroadcastRadioServiceHidlTest.java
@@ -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);
diff --git a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/RadioModuleHidlTest.java b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/RadioModuleHidlTest.java
index 48f5a461d631..1f5e77038728 100644
--- a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/RadioModuleHidlTest.java
+++ b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/RadioModuleHidlTest.java
@@ -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));
 
diff --git a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/StartProgramListUpdatesFanoutTest.java b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/StartProgramListUpdatesFanoutTest.java
index e3c9faa601e7..7d604d497984 100644
--- a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/StartProgramListUpdatesFanoutTest.java
+++ b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/StartProgramListUpdatesFanoutTest.java
@@ -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];
diff --git a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/TunerSessionHidlTest.java b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/TunerSessionHidlTest.java
index b7da5d038f77..ff988a21473a 100644
--- a/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/TunerSessionHidlTest.java
+++ b/core/tests/BroadcastRadioTests/src/com/android/server/broadcastradio/hal2/TunerSessionHidlTest.java
@@ -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];
diff --git a/services/core/java/com/android/server/broadcastradio/IRadioServiceHidlImpl.java b/services/core/java/com/android/server/broadcastradio/IRadioServiceHidlImpl.java
index a8e4034e3f86..408fba1bff3b 100644
--- a/services/core/java/com/android/server/broadcastradio/IRadioServiceHidlImpl.java
+++ b/services/core/java/com/android/server/broadcastradio/IRadioServiceHidlImpl.java
@@ -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");
diff --git a/services/core/java/com/android/server/broadcastradio/hal1/BroadcastRadioService.java b/services/core/java/com/android/server/broadcastradio/hal1/BroadcastRadioService.java
index e50c6e8c21b8..fb42c94b56f4 100644
--- a/services/core/java/com/android/server/broadcastradio/hal1/BroadcastRadioService.java
+++ b/services/core/java/com/android/server/broadcastradio/hal1/BroadcastRadioService.java
@@ -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));
diff --git a/services/core/java/com/android/server/broadcastradio/hal2/BroadcastRadioService.java b/services/core/java/com/android/server/broadcastradio/hal2/BroadcastRadioService.java
index 3d6962783f4a..984bf5125582 100644
--- a/services/core/java/com/android/server/broadcastradio/hal2/BroadcastRadioService.java
+++ b/services/core/java/com/android/server/broadcastradio/hal2/BroadcastRadioService.java
@@ -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) {
diff --git a/services/core/java/com/android/server/broadcastradio/hal2/RadioModule.java b/services/core/java/com/android/server/broadcastradio/hal2/RadioModule.java
index cf1b504037bc..0ea5f0fc1d6a 100644
--- a/services/core/java/com/android/server/broadcastradio/hal2/RadioModule.java
+++ b/services/core/java/com/android/server/broadcastradio/hal2/RadioModule.java
@@ -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");
diff --git a/services/core/java/com/android/server/broadcastradio/hal2/TunerSession.java b/services/core/java/com/android/server/broadcastradio/hal2/TunerSession.java
index 12211eed47fd..7afee277fe1c 100644
--- a/services/core/java/com/android/server/broadcastradio/hal2/TunerSession.java
+++ b/services/core/java/com/android/server/broadcastradio/hal2/TunerSession.java
@@ -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);