audio: Retain IBinder for instances with MinSchedulerPolicy
The binder passed to AIBinder_setMinSchedulerPolicy must also be returned to the client, otherwise setting the policy for it does not make any sense. However, server side interface instance classes only hold a weak binder reference. It's the caller of the 'asBinder' method who must retain a strong reference. This reference must be retained past exiting from the method which returns the instance to the client. To solve this issue, add storing of binders along with server object references. These binders get released after the client calls a 'close'/'destroy'-type method to release instance resources. Bug: 205884982 Test: run `atest VtsHalAudioCoreTargetTest` and effect VTS, and grep logcat for 'destroyed after setMinSchedulerPolicy before being parceled' Change-Id: I8b905b85cb8263c85edae8839a126ffe4e4d1e69
This commit is contained in:
parent
2100e6323c
commit
df5feba141
6 changed files with 45 additions and 33 deletions
|
@ -40,12 +40,13 @@ Factory::Factory(const std::string& file) : mConfig(EffectConfig(file)) {
|
|||
}
|
||||
|
||||
Factory::~Factory() {
|
||||
if (auto count = mEffectUuidMap.size()) {
|
||||
if (auto count = mEffectMap.size()) {
|
||||
LOG(ERROR) << __func__ << " remaining " << count
|
||||
<< " effect instances not destroyed indicating resource leak!";
|
||||
for (const auto& it : mEffectUuidMap) {
|
||||
for (const auto& it : mEffectMap) {
|
||||
if (auto spEffect = it.first.lock()) {
|
||||
LOG(ERROR) << __func__ << " erase remaining instance UUID " << it.second.toString();
|
||||
LOG(ERROR) << __func__ << " erase remaining instance UUID "
|
||||
<< it.second.first.toString();
|
||||
destroyEffectImpl(spEffect);
|
||||
}
|
||||
}
|
||||
|
@ -109,9 +110,10 @@ ndk::ScopedAStatus Factory::createEffect(const AudioUuid& in_impl_uuid,
|
|||
return ndk::ScopedAStatus::fromExceptionCode(EX_TRANSACTION_FAILED);
|
||||
}
|
||||
*_aidl_return = effectSp;
|
||||
AIBinder_setMinSchedulerPolicy(effectSp->asBinder().get(), SCHED_NORMAL,
|
||||
ANDROID_PRIORITY_AUDIO);
|
||||
mEffectUuidMap[std::weak_ptr<IEffect>(effectSp)] = in_impl_uuid;
|
||||
ndk::SpAIBinder effectBinder = effectSp->asBinder();
|
||||
AIBinder_setMinSchedulerPolicy(effectBinder.get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO);
|
||||
mEffectMap[std::weak_ptr<IEffect>(effectSp)] =
|
||||
std::make_pair(in_impl_uuid, std::move(effectBinder));
|
||||
LOG(DEBUG) << __func__ << ": instance " << effectSp.get() << " created successfully";
|
||||
return ndk::ScopedAStatus::ok();
|
||||
} else {
|
||||
|
@ -123,9 +125,9 @@ ndk::ScopedAStatus Factory::createEffect(const AudioUuid& in_impl_uuid,
|
|||
|
||||
ndk::ScopedAStatus Factory::destroyEffectImpl(const std::shared_ptr<IEffect>& in_handle) {
|
||||
std::weak_ptr<IEffect> wpHandle(in_handle);
|
||||
// find UUID with key (std::weak_ptr<IEffect>)
|
||||
if (auto uuidIt = mEffectUuidMap.find(wpHandle); uuidIt != mEffectUuidMap.end()) {
|
||||
auto& uuid = uuidIt->second;
|
||||
// find the effect entry with key (std::weak_ptr<IEffect>)
|
||||
if (auto effectIt = mEffectMap.find(wpHandle); effectIt != mEffectMap.end()) {
|
||||
auto& uuid = effectIt->second.first;
|
||||
// find implementation library with UUID
|
||||
if (auto libIt = mEffectLibMap.find(uuid); libIt != mEffectLibMap.end()) {
|
||||
auto& interface = std::get<kMapEntryInterfaceIndex>(libIt->second);
|
||||
|
@ -136,7 +138,7 @@ ndk::ScopedAStatus Factory::destroyEffectImpl(const std::shared_ptr<IEffect>& in
|
|||
LOG(ERROR) << __func__ << ": UUID " << uuid.toString() << " does not exist in libMap!";
|
||||
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
|
||||
}
|
||||
mEffectUuidMap.erase(uuidIt);
|
||||
mEffectMap.erase(effectIt);
|
||||
return ndk::ScopedAStatus::ok();
|
||||
} else {
|
||||
LOG(ERROR) << __func__ << ": instance " << in_handle << " does not exist!";
|
||||
|
@ -146,9 +148,9 @@ ndk::ScopedAStatus Factory::destroyEffectImpl(const std::shared_ptr<IEffect>& in
|
|||
|
||||
// go over the map and cleanup all expired weak_ptrs.
|
||||
void Factory::cleanupEffectMap() {
|
||||
for (auto it = mEffectUuidMap.begin(); it != mEffectUuidMap.end();) {
|
||||
for (auto it = mEffectMap.begin(); it != mEffectMap.end();) {
|
||||
if (nullptr == it->first.lock()) {
|
||||
it = mEffectUuidMap.erase(it);
|
||||
it = mEffectMap.erase(it);
|
||||
} else {
|
||||
++it;
|
||||
}
|
||||
|
|
|
@ -316,7 +316,8 @@ ndk::ScopedAStatus Module::setModuleDebug(
|
|||
ndk::ScopedAStatus Module::getTelephony(std::shared_ptr<ITelephony>* _aidl_return) {
|
||||
if (mTelephony == nullptr) {
|
||||
mTelephony = ndk::SharedRefBase::make<Telephony>();
|
||||
AIBinder_setMinSchedulerPolicy(mTelephony->asBinder().get(), SCHED_NORMAL,
|
||||
mTelephonyBinder = mTelephony->asBinder();
|
||||
AIBinder_setMinSchedulerPolicy(mTelephonyBinder.get(), SCHED_NORMAL,
|
||||
ANDROID_PRIORITY_AUDIO);
|
||||
}
|
||||
*_aidl_return = mTelephony;
|
||||
|
@ -536,8 +537,9 @@ ndk::ScopedAStatus Module::openInputStream(const OpenInputStreamArguments& in_ar
|
|||
if (auto status = stream->init(); !status.isOk()) {
|
||||
return status;
|
||||
}
|
||||
AIBinder_setMinSchedulerPolicy(stream->asBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO);
|
||||
StreamWrapper streamWrapper(stream);
|
||||
AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL,
|
||||
ANDROID_PRIORITY_AUDIO);
|
||||
auto patchIt = mPatches.find(in_args.portConfigId);
|
||||
if (patchIt != mPatches.end()) {
|
||||
streamWrapper.setStreamIsConnected(findConnectedDevices(in_args.portConfigId));
|
||||
|
@ -587,8 +589,9 @@ ndk::ScopedAStatus Module::openOutputStream(const OpenOutputStreamArguments& in_
|
|||
if (auto status = stream->init(); !status.isOk()) {
|
||||
return status;
|
||||
}
|
||||
AIBinder_setMinSchedulerPolicy(stream->asBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO);
|
||||
StreamWrapper streamWrapper(stream);
|
||||
AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL,
|
||||
ANDROID_PRIORITY_AUDIO);
|
||||
auto patchIt = mPatches.find(in_args.portConfigId);
|
||||
if (patchIt != mPatches.end()) {
|
||||
streamWrapper.setStreamIsConnected(findConnectedDevices(in_args.portConfigId));
|
||||
|
@ -935,6 +938,9 @@ ndk::ScopedAStatus Module::updateScreenState(bool in_isTurnedOn) {
|
|||
ndk::ScopedAStatus Module::getSoundDose(std::shared_ptr<ISoundDose>* _aidl_return) {
|
||||
if (mSoundDose == nullptr) {
|
||||
mSoundDose = ndk::SharedRefBase::make<SoundDose>();
|
||||
mSoundDoseBinder = mSoundDose->asBinder();
|
||||
AIBinder_setMinSchedulerPolicy(mSoundDoseBinder.get(), SCHED_NORMAL,
|
||||
ANDROID_PRIORITY_AUDIO);
|
||||
}
|
||||
*_aidl_return = mSoundDose;
|
||||
LOG(DEBUG) << __func__ << ": returning instance of ISoundDose: " << _aidl_return->get();
|
||||
|
|
|
@ -115,6 +115,7 @@ class Module : public BnModule {
|
|||
// Since it is required to return the same instance of the ITelephony, even
|
||||
// if the client has released it on its side, we need to hold it via a strong pointer.
|
||||
std::shared_ptr<ITelephony> mTelephony;
|
||||
ndk::SpAIBinder mTelephonyBinder;
|
||||
// ids of ports created at runtime via 'connectExternalDevice'.
|
||||
std::set<int32_t> mConnectedDevicePorts;
|
||||
Streams mStreams;
|
||||
|
@ -125,6 +126,7 @@ class Module : public BnModule {
|
|||
float mMasterVolume = 1.0f;
|
||||
bool mMicMute = false;
|
||||
std::shared_ptr<ISoundDose> mSoundDose;
|
||||
ndk::SpAIBinder mSoundDoseBinder;
|
||||
};
|
||||
|
||||
} // namespace aidl::android::hardware::audio::core
|
||||
|
|
|
@ -279,8 +279,11 @@ class StreamOut : public StreamCommon<::aidl::android::hardware::audio::common::
|
|||
|
||||
class StreamWrapper {
|
||||
public:
|
||||
explicit StreamWrapper(std::shared_ptr<StreamIn> streamIn) : mStream(streamIn) {}
|
||||
explicit StreamWrapper(std::shared_ptr<StreamOut> streamOut) : mStream(streamOut) {}
|
||||
explicit StreamWrapper(const std::shared_ptr<StreamIn>& streamIn)
|
||||
: mStream(streamIn), mStreamBinder(streamIn->asBinder()) {}
|
||||
explicit StreamWrapper(const std::shared_ptr<StreamOut>& streamOut)
|
||||
: mStream(streamOut), mStreamBinder(streamOut->asBinder()) {}
|
||||
ndk::SpAIBinder getBinder() const { return mStreamBinder; }
|
||||
bool isStreamOpen() const {
|
||||
return std::visit(
|
||||
[](auto&& ws) -> bool {
|
||||
|
@ -301,6 +304,7 @@ class StreamWrapper {
|
|||
|
||||
private:
|
||||
std::variant<std::weak_ptr<StreamIn>, std::weak_ptr<StreamOut>> mStream;
|
||||
ndk::SpAIBinder mStreamBinder;
|
||||
};
|
||||
|
||||
class Streams {
|
||||
|
|
|
@ -96,9 +96,8 @@ class Factory : public BnFactory {
|
|||
|
||||
std::map<aidl::android::media::audio::common::AudioUuid /* implUUID */, DlEntry> mEffectLibMap;
|
||||
|
||||
std::map<std::weak_ptr<IEffect>, aidl::android::media::audio::common::AudioUuid,
|
||||
std::owner_less<>>
|
||||
mEffectUuidMap;
|
||||
typedef std::pair<aidl::android::media::audio::common::AudioUuid, ndk::SpAIBinder> EffectEntry;
|
||||
std::map<std::weak_ptr<IEffect>, EffectEntry, std::owner_less<>> mEffectMap;
|
||||
|
||||
ndk::ScopedAStatus destroyEffectImpl(const std::shared_ptr<IEffect>& in_handle);
|
||||
void cleanupEffectMap();
|
||||
|
|
|
@ -16,6 +16,7 @@
|
|||
|
||||
#include <cstdlib>
|
||||
#include <ctime>
|
||||
#include <utility>
|
||||
|
||||
#include <android-base/logging.h>
|
||||
#include <android/binder_ibinder_platform.h>
|
||||
|
@ -44,19 +45,17 @@ int main() {
|
|||
CHECK_EQ(STATUS_OK, status);
|
||||
|
||||
// Make modules
|
||||
auto moduleDefault = ndk::SharedRefBase::make<Module>(Module::Type::DEFAULT);
|
||||
const std::string moduleDefaultName = std::string() + Module::descriptor + "/default";
|
||||
AIBinder_setMinSchedulerPolicy(moduleDefault->asBinder().get(), SCHED_NORMAL,
|
||||
ANDROID_PRIORITY_AUDIO);
|
||||
status = AServiceManager_addService(moduleDefault->asBinder().get(), moduleDefaultName.c_str());
|
||||
CHECK_EQ(STATUS_OK, status);
|
||||
|
||||
auto moduleRSubmix = ndk::SharedRefBase::make<Module>(Module::Type::R_SUBMIX);
|
||||
const std::string moduleRSubmixName = std::string() + Module::descriptor + "/r_submix";
|
||||
AIBinder_setMinSchedulerPolicy(moduleRSubmix->asBinder().get(), SCHED_NORMAL,
|
||||
ANDROID_PRIORITY_AUDIO);
|
||||
status = AServiceManager_addService(moduleRSubmix->asBinder().get(), moduleRSubmixName.c_str());
|
||||
CHECK_EQ(STATUS_OK, status);
|
||||
auto createModule = [](Module::Type type, const std::string& instance) {
|
||||
auto module = ndk::SharedRefBase::make<Module>(type);
|
||||
ndk::SpAIBinder moduleBinder = module->asBinder();
|
||||
const std::string moduleName = std::string(Module::descriptor).append("/").append(instance);
|
||||
AIBinder_setMinSchedulerPolicy(moduleBinder.get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO);
|
||||
binder_status_t status = AServiceManager_addService(moduleBinder.get(), moduleName.c_str());
|
||||
CHECK_EQ(STATUS_OK, status);
|
||||
return std::make_pair(module, moduleBinder);
|
||||
};
|
||||
auto modules = {createModule(Module::Type::DEFAULT, "default"),
|
||||
createModule(Module::Type::R_SUBMIX, "r_submix")};
|
||||
|
||||
ABinderProcess_joinThreadPool();
|
||||
return EXIT_FAILURE; // should not reach
|
||||
|
|
Loading…
Reference in a new issue