浏览代码

Workaround for a potential deadlock in MusicHandler

Ivan Savenko 2 年之前
父节点
当前提交
588b635d1c
共有 1 个文件被更改,包括 26 次插入4 次删除
  1. 26 4
      client/CMusicHandler.cpp

+ 26 - 4
client/CMusicHandler.cpp

@@ -409,6 +409,8 @@ void CMusicHandler::release()
 
 void CMusicHandler::playMusic(const std::string & musicURI, bool loop, bool fromStart)
 {
+	boost::mutex::scoped_lock guard(mutex);
+
 	if (current && current->isPlaying() && current->isTrack(musicURI))
 		return;
 
@@ -422,6 +424,8 @@ void CMusicHandler::playMusicFromSet(const std::string & musicSet, const std::st
 
 void CMusicHandler::playMusicFromSet(const std::string & whichSet, bool loop, bool fromStart)
 {
+	boost::mutex::scoped_lock guard(mutex);
+
 	auto selectedSet = musicsSet.find(whichSet);
 	if (selectedSet == musicsSet.end())
 	{
@@ -441,8 +445,6 @@ void CMusicHandler::queueNext(std::unique_ptr<MusicEntry> queued)
 	if (!initialized)
 		return;
 
-	boost::mutex::scoped_lock guard(mutex);
-
 	next = std::move(queued);
 
 	if (current.get() == nullptr || !current->stop(1000))
@@ -487,13 +489,32 @@ void CMusicHandler::setVolume(ui32 percent)
 
 void CMusicHandler::musicFinishedCallback()
 {
-	boost::mutex::scoped_lock guard(mutex);
+	// boost::mutex::scoped_lock guard(mutex);
+	// FIXME: WORKAROUND FOR A POTENTIAL DEADLOCK
+	// It is possible for:
+	// 1) SDL thread to call this method on end of playback
+	// 2) VCMI code to call queueNext() method to queue new file
+	// this leads to:
+	// 1) SDL thread waiting to aquire music lock in this method (while keeping internal SDL mutex locked)
+	// 2) VCMI thread waiting to aquire internal SDL mutex (while keeping music mutex locked)
+	// Because of that (and lack of clear way to fix that)
+	// We will try to acquire lock here and if failed - do nothing
+	// This may break music playback till next song is enqued but won't deadlock the game
+
+	if (!mutex.try_lock())
+	{
+		logGlobal->error("Failed to aquire mutex! Unable to restart music!");
+		return;
+	}
 
 	if (current.get() != nullptr)
 	{
 		// if music is looped, play it again
 		if (current->play())
+		{
+			mutex.unlock();
 			return;
+		}
 		else
 			current.reset();
 	}
@@ -503,6 +524,7 @@ void CMusicHandler::musicFinishedCallback()
 		current.reset(next.release());
 		current->play();
 	}
+	mutex.unlock();
 }
 
 MusicEntry::MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped, bool fromStart):
@@ -597,7 +619,7 @@ bool MusicEntry::play()
 
 bool MusicEntry::stop(int fade_ms)
 {
-	if (Mix_PlayingMusic())
+	if (playing)
 	{
 		playing = false;
 		loop = 0;