fixed #16477: [android, audio] Deadlock if preload the same file more than 3 times in preload callback. (#16478)

And added test case.
This commit is contained in:
James Chen 2016-09-01 11:46:27 +08:00 committed by minggo
parent 35f937cd40
commit 01e67f6203
3 changed files with 71 additions and 28 deletions

View File

@ -210,16 +210,16 @@ void AudioPlayerProvider::preloadEffect(const std::string &audioFilePath, const
return; return;
} }
_pcmCacheMutex.lock();
auto&& iter = _pcmCache.find(audioFilePath);
if (iter != _pcmCache.end())
{ {
std::lock_guard<std::mutex> lk(_pcmCacheMutex); ALOGV("preload return from cache: (%s)", audioFilePath.c_str());
auto&& iter = _pcmCache.find(audioFilePath); _pcmCacheMutex.unlock();
if (iter != _pcmCache.end()) cb(true, iter->second);
{ return;
ALOGV("preload return from cache: (%s)", audioFilePath.c_str());
cb(true, iter->second);
return;
}
} }
_pcmCacheMutex.unlock();
auto info = getFileInfo(audioFilePath); auto info = getFileInfo(audioFilePath);
preloadEffect(info, [this, cb, audioFilePath](bool succeed, PcmData data){ preloadEffect(info, [this, cb, audioFilePath](bool succeed, PcmData data){
@ -246,17 +246,17 @@ void AudioPlayerProvider::preloadEffect(const AudioFileInfo &info, const Preload
{ {
std::string audioFilePath = info.url; std::string audioFilePath = info.url;
// 1. First time check, if it wasn't in the cache, goto 2 step
_pcmCacheMutex.lock();
auto&& iter = _pcmCache.find(audioFilePath);
if (iter != _pcmCache.end())
{ {
// 1. First time check, if it wasn't in the cache, goto 2 step ALOGV("1. Return pcm data from cache, url: %s", info.url.c_str());
std::lock_guard<std::mutex> lk(_pcmCacheMutex); _pcmCacheMutex.unlock();
auto&& iter = _pcmCache.find(audioFilePath); cb(true, iter->second);
if (iter != _pcmCache.end()) return;
{
ALOGV("1. Return pcm data from cache, url: %s", info.url.c_str());
cb(true, iter->second);
return;
}
} }
_pcmCacheMutex.unlock();
{ {
// 2. Check whether the audio file is being preloaded, if it has been removed from map just now, // 2. Check whether the audio file is being preloaded, if it has been removed from map just now,
@ -273,17 +273,19 @@ void AudioPlayerProvider::preloadEffect(const AudioFileInfo &info, const Preload
return; return;
} }
{ // 3. Check it in cache again. If it has been removed from map just now, the file is in // 3. Check it in cache again. If it has been removed from map just now, the file is in
// the cache absolutely. // the cache absolutely.
std::lock_guard<std::mutex> lk2(_pcmCacheMutex); _pcmCacheMutex.lock();
auto&& iter = _pcmCache.find(audioFilePath); auto&& iter = _pcmCache.find(audioFilePath);
if (iter != _pcmCache.end()) if (iter != _pcmCache.end())
{ {
ALOGV("2. Return pcm data from cache, url: %s", info.url.c_str()); ALOGV("2. Return pcm data from cache, url: %s", info.url.c_str());
cb(true, iter->second); _pcmCacheMutex.unlock();
return; cb(true, iter->second);
} return;
} }
_pcmCacheMutex.unlock();
PreloadCallbackParam param; PreloadCallbackParam param;
param.callback = cb; param.callback = cb;

View File

@ -43,6 +43,7 @@ AudioEngineTests::AudioEngineTests()
ADD_TEST_CASE(AudioSwitchStateTest); ADD_TEST_CASE(AudioSwitchStateTest);
ADD_TEST_CASE(AudioSmallFileTest); ADD_TEST_CASE(AudioSmallFileTest);
ADD_TEST_CASE(AudioPauseResumeAfterPlay); ADD_TEST_CASE(AudioPauseResumeAfterPlay);
ADD_TEST_CASE(AudioPreloadSameFileMultipleTimes);
} }
namespace { namespace {
@ -189,7 +190,7 @@ AudioEngineTestDemo::AudioEngineTestDemo()
void AudioEngineTestDemo::onExit() void AudioEngineTestDemo::onExit()
{ {
*_isDestroyed = true; *_isDestroyed = true;
AudioEngine::stopAll(); AudioEngine::uncacheAll();
TestCase::onExit(); TestCase::onExit();
} }
@ -830,3 +831,32 @@ std::string AudioPauseResumeAfterPlay::subtitle() const
{ {
return "Should not crash"; return "Should not crash";
} }
/////////////////////////////////////////////////////////////////////////
void AudioPreloadSameFileMultipleTimes::onEnter()
{
AudioEngineTestDemo::onEnter();
for (int i = 0; i < 10; ++i)
{
AudioEngine::preload("audio/SoundEffectsFX009/FX082.mp3", [i](bool isSucceed){
log("111: %d preload %s", i, isSucceed ? "succeed" : "failed");
AudioEngine::preload("audio/SoundEffectsFX009/FX082.mp3", [i](bool isSucceed){
log("222: %d preload %s", i, isSucceed ? "succeed" : "failed");
AudioEngine::preload("audio/SoundEffectsFX009/FX082.mp3", [i](bool isSucceed){
log("333: %d preload %s", i, isSucceed ? "succeed" : "failed");
});
});
});
}
}
std::string AudioPreloadSameFileMultipleTimes::title() const
{
return "Preload same file multiple times";
}
std::string AudioPreloadSameFileMultipleTimes::subtitle() const
{
return "Should not crash";
}

View File

@ -210,4 +210,15 @@ public:
virtual std::string subtitle() const override; virtual std::string subtitle() const override;
}; };
class AudioPreloadSameFileMultipleTimes : public AudioEngineTestDemo
{
public:
CREATE_FUNC(AudioPreloadSameFileMultipleTimes);
virtual void onEnter() override;
virtual std::string title() const override;
virtual std::string subtitle() const override;
};
#endif /* defined(__NEWAUDIOENGINE_TEST_H_) */ #endif /* defined(__NEWAUDIOENGINE_TEST_H_) */