AudioEngine:Fixed crash cause by multiple threads access into a shared unsynchronized data.[iOS/Mac]

cocos/audio/apple/AudioEngine-inl.h

@@ -35,7 +35,9 @@
 #include "AudioPlayer.h"
 
 NS_CC_BEGIN
-    namespace experimental{
+class Scheduler;
+
+namespace experimental{
 #define MAX_AUDIOINSTANCES 24
 
 class AudioEngineImpl : public cocos2d::Ref
@@ -50,7 +52,7 @@ public:
     void setLoop(int audioID, bool loop);
     bool pause(int audioID);
     bool resume(int audioID);
-    bool stop(int audioID);
+    void stop(int audioID);
     void stopAll();
     float getDuration(int audioID);
     float getCurrentTime(int audioID);
@@ -74,17 +76,13 @@ private:
     std::unordered_map<std::string, AudioCache> _audioCaches;
     
     //audioID,AudioInfo
-    std::unordered_map<int, AudioPlayer>  _audioPlayers;
+    std::unordered_map<int, AudioPlayer*>  _audioPlayers;
-    
     std::mutex _threadMutex;
     
-    std::vector<AudioCache*> _toRemoveCaches;
-    std::vector<int> _toRemoveAudioIDs;
-    
     bool _lazyInitLoop;
     
     int _currentAudioID;
-    
+    Scheduler* _scheduler;
 };
 }
 NS_CC_END

cocos/audio/apple/AudioEngine-inl.mm

@@ -183,7 +183,7 @@ bool AudioEngineImpl::init()
             for (int i = 0; i < MAX_AUDIOINSTANCES; ++i) {
                 _alSourceUsed[_alSources[i]] = false;
             }
-            
+            _scheduler = Director::getInstance()->getScheduler();
             ret = true;
         }
     }while (false);
@@ -233,24 +233,31 @@ int AudioEngineImpl::play2d(const std::string &filePath ,bool loop ,float volume
         return AudioEngine::INVALID_AUDIO_ID;
     }
     
-    AudioCache* audioCache = preload(filePath, nullptr);
+    auto player = new (std::nothrow) AudioPlayer;
-    if (audioCache == nullptr) {
+    if (player == nullptr) {
         return AudioEngine::INVALID_AUDIO_ID;
     }
-    
-    auto player = &_audioPlayers[_currentAudioID];
     player->_alSource = alSource;
     player->_loop = loop;
     player->_volume = volume;
+    
+    auto audioCache = preload(filePath, nullptr);
+    if (audioCache == nullptr) {
+        delete player;
+        return AudioEngine::INVALID_AUDIO_ID;
+    }
+    
+    _threadMutex.lock();
+    _audioPlayers[_currentAudioID] = player;
+    _threadMutex.unlock();
+    
     audioCache->addPlayCallback(std::bind(&AudioEngineImpl::_play2d,this,audioCache,_currentAudioID));
     
     _alSourceUsed[alSource] = true;
     
     if (_lazyInitLoop) {
         _lazyInitLoop = false;
-        
+        _scheduler->schedule(CC_SCHEDULE_SELECTOR(AudioEngineImpl::update), this, 0.05f, false);
-        auto scheduler = cocos2d::Director::getInstance()->getScheduler();
-        scheduler->schedule(CC_SCHEDULE_SELECTOR(AudioEngineImpl::update), this, 0.05f, false);
     }
     
     return _currentAudioID++;
@@ -259,33 +266,26 @@ int AudioEngineImpl::play2d(const std::string &filePath ,bool loop ,float volume
 void AudioEngineImpl::_play2d(AudioCache *cache, int audioID)
 {
     if(cache->_alBufferReady){
-        auto playerIt = _audioPlayers.find(audioID);
-        if (playerIt != _audioPlayers.end()) {
-            if (playerIt->second.play2d(cache)) {
-                AudioEngine::_audioIDInfoMap[audioID].state = AudioEngine::AudioState::PLAYING;
-            }
-            else{
-                _threadMutex.lock();
-                _toRemoveAudioIDs.push_back(audioID);
-                _threadMutex.unlock();
-            }
-        }
-    }
-    else {
         _threadMutex.lock();
-        _toRemoveCaches.push_back(cache);
+        auto playerIt = _audioPlayers.find(audioID);
-        _toRemoveAudioIDs.push_back(audioID);
+        if (playerIt != _audioPlayers.end() && playerIt->second->play2d(cache)) {
+            _scheduler->performFunctionInCocosThread([audioID](){
+                if (AudioEngine::_audioIDInfoMap.find(audioID) != AudioEngine::_audioIDInfoMap.end()) {
+                    AudioEngine::_audioIDInfoMap[audioID].state = AudioEngine::AudioState::PLAYING;
+                }
+            });
+        }
         _threadMutex.unlock();
     }
 }
 
 void AudioEngineImpl::setVolume(int audioID,float volume)
 {
-    auto& player = _audioPlayers[audioID];
+    auto player = _audioPlayers[audioID];
-    player._volume = volume;
+    player->_volume = volume;
     
-    if (player._ready) {
+    if (player->_ready) {
-        alSourcef(_audioPlayers[audioID]._alSource, AL_GAIN, volume);
+        alSourcef(_audioPlayers[audioID]->_alSource, AL_GAIN, volume);
         
         auto error = alGetError();
         if (error != AL_NO_ERROR) {
@@ -296,16 +296,16 @@ void AudioEngineImpl::setVolume(int audioID,float volume)
 
 void AudioEngineImpl::setLoop(int audioID, bool loop)
 {
-    auto& player = _audioPlayers[audioID];
+    auto player = _audioPlayers[audioID];
     
-    if (player._ready) {
+    if (player->_ready) {
-        if (player._streamingSource) {
+        if (player->_streamingSource) {
-            player.setLoop(loop);
+            player->setLoop(loop);
         } else {
             if (loop) {
-                alSourcei(player._alSource, AL_LOOPING, AL_TRUE);
+                alSourcei(player->_alSource, AL_LOOPING, AL_TRUE);
             } else {
-                alSourcei(player._alSource, AL_LOOPING, AL_FALSE);
+                alSourcei(player->_alSource, AL_LOOPING, AL_FALSE);
             }
             
             auto error = alGetError();
@@ -315,14 +315,14 @@ void AudioEngineImpl::setLoop(int audioID, bool loop)
         }
     }
     else {
-        player._loop = loop;
+        player->_loop = loop;
     }
 }
 
 bool AudioEngineImpl::pause(int audioID)
 {
     bool ret = true;
-    alSourcePause(_audioPlayers[audioID]._alSource);
+    alSourcePause(_audioPlayers[audioID]->_alSource);
     
     auto error = alGetError();
     if (error != AL_NO_ERROR) {
@@ -336,7 +336,7 @@ bool AudioEngineImpl::pause(int audioID)
 bool AudioEngineImpl::resume(int audioID)
 {
     bool ret = true;
-    alSourcePlay(_audioPlayers[audioID]._alSource);
+    alSourcePlay(_audioPlayers[audioID]->_alSource);
     
     auto error = alGetError();
     if (error != AL_NO_ERROR) {
@@ -347,45 +347,30 @@ bool AudioEngineImpl::resume(int audioID)
     return ret;
 }
 
-bool AudioEngineImpl::stop(int audioID)
+void AudioEngineImpl::stop(int audioID)
 {
-    bool ret = true;
+    auto player = _audioPlayers[audioID];
-    auto& player = _audioPlayers[audioID];
+    player->destroy();
-    if (player._ready) {
+    _alSourceUsed[player->_alSource] = false;
-        alSourceStop(player._alSource);
-        
-        auto error = alGetError();
-        if (error != AL_NO_ERROR) {
-            ret = false;
-            printf("%s: audio id = %d, error = %x\n", __PRETTY_FUNCTION__,audioID,error);
-        }
-    }
-    
-    alSourcei(player._alSource, AL_BUFFER, 0);
-    
-    _alSourceUsed[player._alSource] = false;
-    _audioPlayers.erase(audioID);
-    
-    return ret;
 }
 
 void AudioEngineImpl::stopAll()
 {
+    for(auto&& player : _audioPlayers)
+    {
+        player.second->destroy();
+    }
     for(int index = 0; index < MAX_AUDIOINSTANCES; ++index)
     {
-        alSourceStop(_alSources[index]);
-        alSourcei(_alSources[index], AL_BUFFER, 0);
         _alSourceUsed[_alSources[index]] = false;
     }
-    
-    _audioPlayers.clear();
 }
 
 float AudioEngineImpl::getDuration(int audioID)
 {
-    auto& player = _audioPlayers[audioID];
+    auto player = _audioPlayers[audioID];
-    if(player._ready){
+    if(player->_ready){
-        return player._audioCache->_duration;
+        return player->_audioCache->_duration;
     } else {
         return AudioEngine::TIME_UNKNOWN;
     }
@@ -394,12 +379,12 @@ float AudioEngineImpl::getDuration(int audioID)
 float AudioEngineImpl::getCurrentTime(int audioID)
 {
     float ret = 0.0f;
-    auto& player = _audioPlayers[audioID];
+    auto player = _audioPlayers[audioID];
-    if(player._ready){
+    if(player->_ready){
-        if (player._streamingSource) {
+        if (player->_streamingSource) {
-            ret = player.getTime();
+            ret = player->getTime();
         } else {
-            alGetSourcef(player._alSource, AL_SEC_OFFSET, &ret);
+            alGetSourcef(player->_alSource, AL_SEC_OFFSET, &ret);
             
             auto error = alGetError();
             if (error != AL_NO_ERROR) {
@@ -414,25 +399,25 @@ float AudioEngineImpl::getCurrentTime(int audioID)
 bool AudioEngineImpl::setCurrentTime(int audioID, float time)
 {
     bool ret = false;
-    auto& player = _audioPlayers[audioID];
+    auto player = _audioPlayers[audioID];
     
     do {
-        if (!player._ready) {
+        if (!player->_ready) {
             break;
         }
         
-        if (player._streamingSource) {
+        if (player->_streamingSource) {
-            ret = player.setTime(time);
+            ret = player->setTime(time);
             break;
         }
         else {
-            if (player._audioCache->_bytesOfRead != player._audioCache->_dataSize &&
+            if (player->_audioCache->_bytesOfRead != player->_audioCache->_dataSize &&
-                (time * player._audioCache->_sampleRate * player._audioCache->_bytesPerFrame) > player._audioCache->_bytesOfRead) {
+                (time * player->_audioCache->_sampleRate * player->_audioCache->_bytesPerFrame) > player->_audioCache->_bytesOfRead) {
                 printf("%s: audio id = %d\n", __PRETTY_FUNCTION__,audioID);
                 break;
             }
             
-            alSourcef(player._alSource, AL_SEC_OFFSET, time);
+            alSourcef(player->_alSource, AL_SEC_OFFSET, time);
             
             auto error = alGetError();
             if (error != AL_NO_ERROR) {
@@ -447,57 +432,40 @@ bool AudioEngineImpl::setCurrentTime(int audioID, float time)
 
 void AudioEngineImpl::setFinishCallback(int audioID, const std::function<void (int, const std::string &)> &callback)
 {
-    _audioPlayers[audioID]._finishCallbak = callback;
+    _audioPlayers[audioID]->_finishCallbak = callback;
 }
 
 void AudioEngineImpl::update(float dt)
 {
     ALint sourceState;
     int audioID;
-    
+    AudioPlayer* player;
-    if (_threadMutex.try_lock()) {
-        size_t removeAudioCount = _toRemoveAudioIDs.size();
-        for (size_t index = 0; index < removeAudioCount; ++index) {
-            audioID = _toRemoveAudioIDs[index];
-            auto playerIt = _audioPlayers.find(audioID);
-            if (playerIt != _audioPlayers.end()) {
-                _alSourceUsed[playerIt->second._alSource] = false;
-                if(playerIt->second._finishCallbak) {
-                    auto& audioInfo = AudioEngine::_audioIDInfoMap[audioID];
-                    playerIt->second._finishCallbak(audioID, *audioInfo.filePath);
-                }
-                _audioPlayers.erase(audioID);
-                AudioEngine::remove(audioID);
-            }
-        }
-        size_t removeCacheCount = _toRemoveCaches.size();
-        for (size_t index = 0; index < removeCacheCount; ++index) {
-            auto itEnd = _audioCaches.end();
-            for (auto it = _audioCaches.begin(); it != itEnd; ++it) {
-                if (&it->second == _toRemoveCaches[index]) {
-                    _audioCaches.erase(it);
-                    break;
-                }
-            }
-        }
-        _threadMutex.unlock();
-    }
     
     for (auto it = _audioPlayers.begin(); it != _audioPlayers.end(); ) {
         audioID = it->first;
-        auto& player = it->second;
+        player = it->second;
-        alGetSourcei(player._alSource, AL_SOURCE_STATE, &sourceState);
+        alGetSourcei(player->_alSource, AL_SOURCE_STATE, &sourceState);
         
-        if (player._ready && sourceState == AL_STOPPED) {
+        if(player->_removeByAudioEngine)
-            _alSourceUsed[player._alSource] = false;
+        {
-            if (player._finishCallbak) {
+            AudioEngine::remove(audioID);
+            _threadMutex.lock();
+            it = _audioPlayers.erase(it);
+            _threadMutex.unlock();
+            delete player;
+        }
+        else if (player->_ready && sourceState == AL_STOPPED) {
+            _alSourceUsed[player->_alSource] = false;
+            if (player->_finishCallbak) {
                 auto& audioInfo = AudioEngine::_audioIDInfoMap[audioID];
-                player._finishCallbak(audioID, *audioInfo.filePath);
+                player->_finishCallbak(audioID, *audioInfo.filePath);
             }
             
             AudioEngine::remove(audioID);
-            
+            delete player;
+            _threadMutex.lock();
             it = _audioPlayers.erase(it);
+            _threadMutex.unlock();
         }
         else{
             ++it;
@@ -506,9 +474,7 @@ void AudioEngineImpl::update(float dt)
     
     if(_audioPlayers.empty()){
         _lazyInitLoop = true;
-        
+        _scheduler->unschedule(CC_SCHEDULE_SELECTOR(AudioEngineImpl::update), this);
-        auto scheduler = cocos2d::Director::getInstance()->getScheduler();
-        scheduler->unschedule(CC_SCHEDULE_SELECTOR(AudioEngineImpl::update), this);
     }
 }
 

cocos/audio/apple/AudioPlayer.h

@@ -47,6 +47,8 @@ public:
     AudioPlayer();
     ~AudioPlayer();
     
+    void destroy();
+    
     //queue buffer related stuff
     bool setTime(float time);
     float getTime() { return _currTime;}
@@ -62,6 +64,8 @@ protected:
     bool _loop;
     std::function<void (int, const std::string &)> _finishCallbak;
     
+    bool _beDestroy;
+    bool _removeByAudioEngine;
     bool _ready;
     ALuint _alSource;
     
@@ -72,7 +76,6 @@ protected:
     std::thread _rotateBufferThread;
     std::condition_variable _sleepCondition;
     std::mutex _sleepMutex;
-    bool _exitThread;
     bool _timeDirty;
     
     friend class AudioEngineImpl;

cocos/audio/apple/AudioPlayer.mm

@@ -38,19 +38,20 @@ using namespace cocos2d::experimental;
 AudioPlayer::AudioPlayer()
 : _audioCache(nullptr)
 , _finishCallbak(nullptr)
+, _beDestroy(false)
+, _removeByAudioEngine(false)
 , _ready(false)
 , _currTime(0.0f)
 , _streamingSource(false)
-, _exitThread(false)
 , _timeDirty(false)
 {    
 }
 
 AudioPlayer::~AudioPlayer()
 {
-    _exitThread = true;
+    if (_streamingSource) {
-    if (_audioCache && _audioCache->_queBufferFrames > 0) {
+        _beDestroy = true;
-        _sleepCondition.notify_all();
+        _sleepCondition.notify_one();
         if (_rotateBufferThread.joinable()) {
             _rotateBufferThread.join();
         }
@@ -58,9 +59,26 @@ AudioPlayer::~AudioPlayer()
     }
 }
 
+void AudioPlayer::destroy()
+{
+    alSourceStop(_alSource);
+    alSourcei(_alSource, AL_BUFFER, NULL);
+    
+    std::unique_lock<std::mutex> lk(_sleepMutex);
+    _beDestroy = true;
+    if (_streamingSource) {
+        _sleepCondition.notify_one();
+    }
+    else if (_ready) {
+        _removeByAudioEngine = true;
+    }
+    _ready = false;
+}
+
 bool AudioPlayer::play2d(AudioCache* cache)
 {
     if (!cache->_alBufferReady) {
+        _removeByAudioEngine = true;
         return false;
     }
     _audioCache = cache;
@@ -74,36 +92,49 @@ bool AudioPlayer::play2d(AudioCache* cache)
         if (_loop) {
             alSourcei(_alSource, AL_LOOPING, AL_TRUE);
         }
-        alSourcei(_alSource, AL_BUFFER, _audioCache->_alBufferId);
+    }
-        
+    else {
-    } else {
+        alGetError();
-        _streamingSource = true;
-        
-        auto alError = alGetError();
         alGenBuffers(3, _bufferIds);
-        alError = alGetError();
+        auto alError = alGetError();
         if (alError == AL_NO_ERROR) {
-            _rotateBufferThread = std::thread(&AudioPlayer::rotateBufferThread,this, _audioCache->_queBufferFrames * QUEUEBUFFER_NUM + 1);
-            
             for (int index = 0; index < QUEUEBUFFER_NUM; ++index) {
                 alBufferData(_bufferIds[index], _audioCache->_format, _audioCache->_queBuffers[index], _audioCache->_queBufferSize[index], _audioCache->_sampleRate);
             }
-            alSourceQueueBuffers(_alSource, QUEUEBUFFER_NUM, _bufferIds);
         }
         else {
             printf("%s:alGenBuffers error code:%x", __PRETTY_FUNCTION__,alError);
+            _removeByAudioEngine = true;
             return false;
         }
+        _streamingSource = true;
     }
     
-    alSourcePlay(_alSource);
+    {
-    _ready = true;
+        std::unique_lock<std::mutex> lk(_sleepMutex);
-    auto alError = alGetError();
+        if (_beDestroy) {
+            _removeByAudioEngine = true;
+            return false;
+        }
+        if (_streamingSource) {
+            alSourceQueueBuffers(_alSource, QUEUEBUFFER_NUM, _bufferIds);
+            _rotateBufferThread = std::thread(&AudioPlayer::rotateBufferThread,this, _audioCache->_queBufferFrames * QUEUEBUFFER_NUM + 1);
+        }
+        else {
+            alSourcei(_alSource, AL_BUFFER, _audioCache->_alBufferId);
+        }
+        
+        alGetError();
+        alSourcePlay(_alSource);
+    }
     
+    auto alError = alGetError();
     if (alError != AL_NO_ERROR) {
         printf("%s:alSourcePlay error code:%x\n", __PRETTY_FUNCTION__,alError);
+        _removeByAudioEngine = true;
         return false;
     }
+    _ready = true;
     
     return true;
 }
@@ -137,7 +168,7 @@ void AudioPlayer::rotateBufferThread(int offsetFrame)
         ExtAudioFileSeek(extRef, offsetFrame);
     }
     
-    while (!_exitThread) {
+    while (!_beDestroy) {
         alGetSourcei(_alSource, AL_SOURCE_STATE, &sourceState);
         if (sourceState == AL_PLAYING) {
             alGetSourcei(_alSource, AL_BUFFERS_PROCESSED, &bufferProcessed);
@@ -168,7 +199,7 @@ void AudioPlayer::rotateBufferThread(int offsetFrame)
                         theDataBuffer.mBuffers[0].mDataByteSize = _audioCache->_queBufferBytes;
                         ExtAudioFileRead(extRef, (UInt32*)&frames, &theDataBuffer);
                     } else {
-                        _exitThread = true;
+                        _beDestroy = true;
                         break;
                     }
                 }
@@ -180,10 +211,11 @@ void AudioPlayer::rotateBufferThread(int offsetFrame)
             }
         }
         
-        if (_exitThread) {
+        std::unique_lock<std::mutex> lk(_sleepMutex);
+        if (_beDestroy) {
             break;
         }
-        std::unique_lock<std::mutex> lk(_sleepMutex);
+        
         _sleepCondition.wait_for(lk,std::chrono::milliseconds(75));
     }
     
@@ -198,7 +230,7 @@ ExitBufferThread:
 
 bool AudioPlayer::setLoop(bool loop)
 {
-    if (!_exitThread ) {
+    if (!_beDestroy ) {
         _loop = loop;
         return true;
     }
@@ -208,7 +240,7 @@ bool AudioPlayer::setLoop(bool loop)
 
 bool AudioPlayer::setTime(float time)
 {
-    if (!_exitThread && time >= 0.0f && time < _audioCache->_duration) {
+    if (!_beDestroy && time >= 0.0f && time < _audioCache->_duration) {
         
         _currTime = time;
         _timeDirty = true;