From a276f7ed088b20fdd83e5f9bb6aa45fe8460c041 Mon Sep 17 00:00:00 2001 From: George Lu Date: Wed, 27 Mar 2019 15:36:53 -0700 Subject: [PATCH] Fix for b/126739800: Enforce aggressive open policy on TunerSessions. BroadcastRadioService currently enforces a 1 ITuner (AIDL) to 1 ITunerSession (HAL) relationship, so because ITunerSession.openSession() mandates an "aggressive open" policy, a similar policy must be enforced on ITuners. Failure to apply this policy resulted in RadioAppService holding onto an invalid ITuner, which caused the radio app to crash when it tried to use it. When an ITuner is closed due to the "aggressive open" policy, that client is notified through a callback to RadioTuner.Callback.onError(ERROR_HARDWARE_FAILURE). Fixes: 126739800 Test: Manually verified radio app usable after switching users Change-Id: I8769f7c41c8f92f024666341ff02e1fe9f137acc --- .../hal2/BroadcastRadioService.java | 37 +++++++++++++++---- .../broadcastradio/hal2/TunerSession.java | 15 ++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) 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 b8810c8f379f9..2df89821611c8 100644 --- a/services/core/java/com/android/server/broadcastradio/hal2/BroadcastRadioService.java +++ b/services/core/java/com/android/server/broadcastradio/hal2/BroadcastRadioService.java @@ -24,6 +24,7 @@ import android.hardware.radio.ICloseHandle; import android.hardware.radio.ITuner; import android.hardware.radio.ITunerCallback; import android.hardware.radio.RadioManager; +import android.hardware.radio.RadioTuner; import android.hidl.manager.V1_0.IServiceManager; import android.hidl.manager.V1_0.IServiceNotification; import android.os.IHwBinder.DeathRecipient; @@ -49,9 +50,17 @@ public class BroadcastRadioService { @GuardedBy("mLock") private final Map mServiceNameToModuleIdMap = new HashMap<>(); + // Map from module ID to RadioModule created by mServiceListener.onRegistration(). @GuardedBy("mLock") private final Map mModules = new HashMap<>(); + // Map from module ID to TunerSession created by openSession(). + // + // Because this service currently implements a 1 AIDL to 1 HAL policy, mTunerSessions is used to + // enforce the "aggresive open" policy mandated for IBroadcastRadio.openSession(). In the + // future, this solution will be replaced with a multiple-AIDL to 1 HAL implementation. + private final Map mTunerSessions = new HashMap<>(); + private IServiceNotification.Stub mServiceListener = new IServiceNotification.Stub() { @Override public void onRegistration(String fqName, String serviceName, boolean preexisting) { @@ -72,6 +81,7 @@ public class BroadcastRadioService { } Slog.v(TAG, "loaded broadcast radio module " + moduleId + ": " + serviceName + " (HAL 2.0)"); + closeTunerSessionLocked(moduleId); mModules.put(moduleId, module); if (newService) { @@ -96,6 +106,7 @@ public class BroadcastRadioService { synchronized (mLock) { int moduleId = (int) cookie; mModules.remove(moduleId); + closeTunerSessionLocked(moduleId); for (Map.Entry entry : mServiceNameToModuleIdMap.entrySet()) { if (entry.getValue() == moduleId) { @@ -152,16 +163,20 @@ public class BroadcastRadioService { RadioModule module = null; synchronized (mLock) { module = mModules.get(moduleId); - } - if (module == null) { - throw new IllegalArgumentException("Invalid module ID"); + if (module == null) { + throw new IllegalArgumentException("Invalid module ID"); + } + closeTunerSessionLocked(moduleId); } - TunerSession session = module.openSession(callback); - if (legacyConfig != null) { - session.setConfiguration(legacyConfig); + TunerSession tunerSession = module.openSession(callback); + synchronized (mLock) { + mTunerSessions.put(moduleId, tunerSession); } - return session; + if (legacyConfig != null) { + tunerSession.setConfiguration(legacyConfig); + } + return tunerSession; } public ICloseHandle addAnnouncementListener(@NonNull int[] enabledTypes, @@ -183,4 +198,12 @@ public class BroadcastRadioService { } return aggregator; } + + private void closeTunerSessionLocked(int moduleId) { + TunerSession tunerSession = mTunerSessions.remove(moduleId); + if (tunerSession != null) { + Slog.d(TAG, "Closing previous TunerSession"); + tunerSession.close(RadioTuner.ERROR_HARDWARE_FAILURE); + } + } } 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 9833507f8fced..05ca144ed3b95 100644 --- a/services/core/java/com/android/server/broadcastradio/hal2/TunerSession.java +++ b/services/core/java/com/android/server/broadcastradio/hal2/TunerSession.java @@ -17,6 +17,7 @@ package com.android.server.broadcastradio.hal2; import android.annotation.NonNull; +import android.annotation.Nullable; import android.graphics.Bitmap; import android.hardware.broadcastradio.V2_0.ConfigFlag; import android.hardware.broadcastradio.V2_0.ITunerSession; @@ -58,8 +59,22 @@ class TunerSession extends ITuner.Stub { @Override public void close() { + close(null); + } + + /** + * Closes the TunerSession. If error is non-null, the client's onError() callback is invoked + * first with the specified error, see {@link + * android.hardware.radio.RadioTuner.Callback#onError}. + * + * @param error Optional error to send to client before session is closed. + */ + public void close(@Nullable Integer error) { synchronized (mLock) { if (mIsClosed) return; + if (error != null) { + TunerCallback.dispatch(() -> mCallback.mClientCb.onError(error)); + } mIsClosed = true; } }