Make EffectFactory implementation thread-safe
Also adjust some log level as verbos
Bug: 271500140
Test: atest --test-mapping hardware/interfaces/audio/aidl/vts:presubmit
Change-Id: I04560c62bdbcfb85dbe223bec0149b112205a323
diff --git a/audio/aidl/default/EffectFactory.cpp b/audio/aidl/default/EffectFactory.cpp
index 62f3c7e..96f13ba 100644
--- a/audio/aidl/default/EffectFactory.cpp
+++ b/audio/aidl/default/EffectFactory.cpp
@@ -49,18 +49,18 @@
if (auto spEffect = it.first.lock()) {
LOG(ERROR) << __func__ << " erase remaining instance UUID "
<< ::android::audio::utils::toString(it.second.first);
- destroyEffectImpl(spEffect);
+ destroyEffectImpl_l(spEffect);
}
}
}
}
-ndk::ScopedAStatus Factory::getDescriptorWithUuid(const AudioUuid& uuid, Descriptor* desc) {
+ndk::ScopedAStatus Factory::getDescriptorWithUuid_l(const AudioUuid& uuid, Descriptor* desc) {
RETURN_IF(!desc, EX_NULL_POINTER, "nullDescriptor");
if (mEffectLibMap.count(uuid)) {
auto& entry = mEffectLibMap[uuid];
- getDlSyms(entry);
+ getDlSyms_l(entry);
auto& libInterface = std::get<kMapEntryInterfaceIndex>(entry);
RETURN_IF(!libInterface || !libInterface->queryEffectFunc, EX_NULL_POINTER,
"dlNullQueryEffectFunc");
@@ -75,6 +75,7 @@
const std::optional<AudioUuid>& in_impl_uuid,
const std::optional<AudioUuid>& in_proxy_uuid,
std::vector<Descriptor>* _aidl_return) {
+ std::lock_guard lg(mMutex);
// get the matching list
std::vector<Descriptor::Identity> idList;
std::copy_if(mIdentitySet.begin(), mIdentitySet.end(), std::back_inserter(idList),
@@ -88,7 +89,8 @@
for (const auto& id : idList) {
if (mEffectLibMap.count(id.uuid)) {
Descriptor desc;
- RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid(id.uuid, &desc), "getDescriptorFailed");
+ RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid_l(id.uuid, &desc),
+ "getDescriptorFailed");
// update proxy UUID with information from config xml
desc.common.id.proxy = id.proxy;
_aidl_return->emplace_back(std::move(desc));
@@ -99,6 +101,7 @@
ndk::ScopedAStatus Factory::queryProcessing(const std::optional<Processing::Type>& in_type,
std::vector<Processing>* _aidl_return) {
+ std::lock_guard lg(mMutex);
const auto& processings = mConfig.getProcessingMap();
// Processing stream type
for (const auto& procIter : processings) {
@@ -110,7 +113,7 @@
if (libs.proxyLibrary.has_value()) {
desc.common.id.proxy = libs.proxyLibrary.value().uuid;
}
- RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid(lib.uuid, &desc),
+ RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid_l(lib.uuid, &desc),
"getDescriptorFailed");
process.ids.emplace_back(desc);
}
@@ -125,9 +128,10 @@
ndk::ScopedAStatus Factory::createEffect(const AudioUuid& in_impl_uuid,
std::shared_ptr<IEffect>* _aidl_return) {
LOG(DEBUG) << __func__ << ": UUID " << ::android::audio::utils::toString(in_impl_uuid);
+ std::lock_guard lg(mMutex);
if (mEffectLibMap.count(in_impl_uuid)) {
auto& entry = mEffectLibMap[in_impl_uuid];
- getDlSyms(entry);
+ getDlSyms_l(entry);
auto& libInterface = std::get<kMapEntryInterfaceIndex>(entry);
RETURN_IF(!libInterface || !libInterface->createEffectFunc, EX_NULL_POINTER,
@@ -152,7 +156,7 @@
return ndk::ScopedAStatus::ok();
}
-ndk::ScopedAStatus Factory::destroyEffectImpl(const std::shared_ptr<IEffect>& in_handle) {
+ndk::ScopedAStatus Factory::destroyEffectImpl_l(const std::shared_ptr<IEffect>& in_handle) {
std::weak_ptr<IEffect> wpHandle(in_handle);
// find the effect entry with key (std::weak_ptr<IEffect>)
if (auto effectIt = mEffectMap.find(wpHandle); effectIt != mEffectMap.end()) {
@@ -177,7 +181,7 @@
}
// go over the map and cleanup all expired weak_ptrs.
-void Factory::cleanupEffectMap() {
+void Factory::cleanupEffectMap_l() {
for (auto it = mEffectMap.begin(); it != mEffectMap.end();) {
if (nullptr == it->first.lock()) {
it = mEffectMap.erase(it);
@@ -189,13 +193,15 @@
ndk::ScopedAStatus Factory::destroyEffect(const std::shared_ptr<IEffect>& in_handle) {
LOG(DEBUG) << __func__ << ": instance " << in_handle.get();
- ndk::ScopedAStatus status = destroyEffectImpl(in_handle);
+ std::lock_guard lg(mMutex);
+ ndk::ScopedAStatus status = destroyEffectImpl_l(in_handle);
// always do the cleanup
- cleanupEffectMap();
+ cleanupEffectMap_l();
return status;
}
-bool Factory::openEffectLibrary(const AudioUuid& impl, const std::string& path) {
+bool Factory::openEffectLibrary(const AudioUuid& impl,
+ const std::string& path) NO_THREAD_SAFETY_ANALYSIS {
std::function<void(void*)> dlClose = [](void* handle) -> void {
if (handle && dlclose(handle)) {
LOG(ERROR) << "dlclose failed " << dlerror();
@@ -219,9 +225,9 @@
return true;
}
-void Factory::createIdentityWithConfig(const EffectConfig::Library& configLib,
- const AudioUuid& typeUuid,
- const std::optional<AudioUuid> proxyUuid) {
+void Factory::createIdentityWithConfig(
+ const EffectConfig::Library& configLib, const AudioUuid& typeUuid,
+ const std::optional<AudioUuid> proxyUuid) NO_THREAD_SAFETY_ANALYSIS {
static const auto& libMap = mConfig.getLibraryMap();
const std::string& libName = configLib.name;
if (auto path = libMap.find(libName); path != libMap.end()) {
@@ -263,7 +269,7 @@
}
}
-void Factory::getDlSyms(DlEntry& entry) {
+void Factory::getDlSyms_l(DlEntry& entry) {
auto& dlHandle = std::get<kMapEntryHandleIndex>(entry);
RETURN_VALUE_IF(!dlHandle, void(), "dlNullHandle");
// Get the reference of the DL interfaces in library map tuple.