Skip to content
This repository
Browse code

Fixing second reported bug in #13574 - apparently gcc overoptimizes c…

…ritsections/checks.
  • Loading branch information...
commit a6058c0148c97178140761135931893fc6303f5b 1 parent 5e98e17
authored December 25, 2012
7  xbmc/music/karaoke/karaokelyricsmanager.cpp
@@ -109,12 +109,15 @@ bool CKaraokeLyricsManager::Start(const CStdString & strSongPath)
109 109
 
110 110
 void CKaraokeLyricsManager::Stop()
111 111
 {
112  
-  CSingleLock lock (m_CritSection);
  112
+  m_CritSection.lock();
113 113
 
114 114
   m_karaokeSongPlaying = false;
115 115
 
116 116
   if ( !m_Lyrics )
  117
+  {
  118
+    m_CritSection.unlock();
117 119
     return;
  120
+  }
118 121
 
119 122
   // Clean up and close karaoke window when stopping
120 123
   CGUIWindowKaraokeLyrics *window = (CGUIWindowKaraokeLyrics*) g_windowManager.GetWindow(WINDOW_KARAOKELYRICS);
@@ -129,6 +132,8 @@ void CKaraokeLyricsManager::Stop()
129 132
   m_Lyrics->Shutdown();
130 133
   delete m_Lyrics;
131 134
   m_Lyrics = 0;
  135
+  
  136
+  m_CritSection.unlock();
132 137
 }
133 138
 
134 139
 

5 notes on commit a6058c0

davilla
Collaborator

this fix is disturbing, we do "CSingleLock lock (m_CritSection);" all over the place.

Jim Carroll
Collaborator

If gcc optimizes this out we're in real trouble. This is a typical use of the "guard pattern" and it's used all over the place. Not only MUST the constructor and the destructor be called, this is the typical way to write exception safe code and so the destructor needs to be called - in the right order - when the stack is unwound.

davilla
Collaborator

seriously, what exactly does this fix.

arnova
Collaborator

Dude, really start using PR's for this shit, especially so close to release.

Joakim Plate
Collaborator
Please sign in to comment.
Something went wrong with that request. Please try again.