From 5fb74d581fbd03748aa94782645dec2e14e19b4d Mon Sep 17 00:00:00 2001 From: sesse Date: Thu, 4 Mar 2010 15:57:08 +0000 Subject: Make emulated Win32 critical sections thread safe. Earlier, cs->locked was accessed outside the mutex to get around the problem that default pthread mutexes are not recursive (ie., you cannot do a double-lock from the same thread), causing a thread-safety problem, as both detected by Helgrind and showing up in some multithreaded codecs. The ideal solution here would be to simply use recursive pthread mutexes, but there were concerns about reduced debuggability and possibly portability. Thus, instead, rewrite the critical sections to be a simple lock count (with owner) protected by a regular mutex. Whenever a thread wants to enter the critical section and lock_count is not 0, it sleeps on a special event that tells it when the critical section is available. git-svn-id: svn://svn.mplayerhq.hu/mplayer/trunk@30837 b3059339-0415-0410-9bf9-f77b7e298cf2 --- loader/win32.c | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) (limited to 'loader') diff --git a/loader/win32.c b/loader/win32.c index f31de74730..2906d14102 100644 --- a/loader/win32.c +++ b/loader/win32.c @@ -350,7 +350,8 @@ struct CRITSECT { pthread_t id; pthread_mutex_t mutex; - int locked; + pthread_cond_t unlocked; + int lock_count; long deadbeef; }; @@ -1331,7 +1332,8 @@ static void WINAPI expInitializeCriticalSection(CRITICAL_SECTION* c) return; } pthread_mutex_init(&cs->mutex, NULL); - cs->locked = 0; + pthread_cond_init(&cs->unlocked, NULL); + cs->lock_count = 0; critsecs_list[i].cs_win = c; critsecs_list[i].cs_unix = cs; dbgprintf("InitializeCriticalSection -> itemno=%d, cs_win=%p, cs_unix=%p\n", @@ -1342,7 +1344,8 @@ static void WINAPI expInitializeCriticalSection(CRITICAL_SECTION* c) struct CRITSECT* cs = mreq_private(sizeof(struct CRITSECT) + sizeof(CRITICAL_SECTION), 0, AREATYPE_CRITSECT); pthread_mutex_init(&cs->mutex, NULL); - cs->locked=0; + pthread_cond_init(&cs->unlocked, NULL); + cs->lock_count = 0; cs->deadbeef = 0xdeadbeef; *(void**)c = cs; } @@ -1374,12 +1377,17 @@ static void WINAPI expEnterCriticalSection(CRITICAL_SECTION* c) #endif dbgprintf("Win32 Warning: Accessed uninitialized Critical Section (%p)!\n", c); } - if(cs->locked) - if(cs->id==pthread_self()) - return; pthread_mutex_lock(&(cs->mutex)); - cs->locked=1; - cs->id=pthread_self(); + if (cs->lock_count > 0 && cs->id == pthread_self()) { + cs->lock_count++; + } else { + while (cs->lock_count != 0) { + pthread_cond_wait(&(cs->unlocked), &(cs->mutex)); + } + cs->lock_count = 1; + cs->id = pthread_self(); + } + pthread_mutex_unlock(&(cs->mutex)); return; } static void WINAPI expLeaveCriticalSection(CRITICAL_SECTION* c) @@ -1396,13 +1404,16 @@ static void WINAPI expLeaveCriticalSection(CRITICAL_SECTION* c) dbgprintf("Win32 Warning: Leaving uninitialized Critical Section %p!!\n", c); return; } - if (cs->locked) - { - cs->locked=0; - pthread_mutex_unlock(&(cs->mutex)); + pthread_mutex_lock(&(cs->mutex)); + if (cs->lock_count == 0) { + dbgprintf("Win32 Warning: Unlocking unlocked Critical Section %p!!\n", c); + } else { + cs->lock_count--; } - else - dbgprintf("Win32 Warning: Unlocking unlocked Critical Section %p!!\n", c); + if (cs->lock_count == 0) { + pthread_cond_signal(&(cs->unlocked)); + } + pthread_mutex_unlock(&(cs->mutex)); return; } @@ -1424,14 +1435,16 @@ static void WINAPI expDeleteCriticalSection(CRITICAL_SECTION *c) return; } - if (cs->locked) + pthread_mutex_lock(&(cs->mutex)); + if (cs->lock_count > 0) { - dbgprintf("Win32 Warning: Deleting unlocked Critical Section %p!!\n", c); - pthread_mutex_unlock(&(cs->mutex)); + dbgprintf("Win32 Warning: Deleting locked Critical Section %p!!\n", c); } + pthread_mutex_unlock(&(cs->mutex)); #ifndef GARBAGE pthread_mutex_destroy(&(cs->mutex)); + pthread_cond_destroy(&(cs->unlocked)); // released by GarbageCollector in my_relase otherwise #endif my_release(cs); -- cgit v1.2.3