[vlc-devel] [PATCH 1/2] win32: implement non-recursive locks

[Steve Lhomme]

locked & b_recursive are always accessed in the CriticalSection

--
simple rebase of https://patches.videolan.org/patch/13641/
---
 include/vlc_threads.h | 12 +++++-------
 src/win32/thread.c    | 30 ++++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/include/vlc_threads.h b/include/vlc_threads.h
index 33b9b53..1fe22bf 100644
--- a/include/vlc_threads.h
+++ b/include/vlc_threads.h
@@ -57,18 +57,16 @@ typedef struct vlc_thread *vlc_thread_t;
 # define LIBVLC_NEED_SLEEP
 typedef struct
 {
+    int locked;
+    bool b_recursive;
     bool dynamic;
     union
     {
-        struct
-        {
-            bool locked;
-            unsigned long contention;
-        };
-        CRITICAL_SECTION mutex;
+        unsigned long contention; /* static */
+        CRITICAL_SECTION mutex;   /* dynamic */
     };
 } vlc_mutex_t;
-#define VLC_STATIC_MUTEX { false, { { false, 0 } } }
+#define VLC_STATIC_MUTEX { 0, false, false, { 0 } }
 #define LIBVLC_NEED_CONDVAR
 #define LIBVLC_NEED_SEMAPHORE
 #define LIBVLC_NEED_RWLOCK
diff --git a/src/win32/thread.c b/src/win32/thread.c
index 9eb3db7..8d53185 100644
--- a/src/win32/thread.c
+++ b/src/win32/thread.c
@@ -96,15 +96,17 @@ static BOOL WINAPI SleepConditionVariableFallback(CONDITION_VARIABLE *cv,
 /*** Mutexes ***/
 void vlc_mutex_init( vlc_mutex_t *p_mutex )
 {
-    /* This creates a recursive mutex. This is OK as fast mutexes have
-     * no defined behavior in case of recursive locking. */
     InitializeCriticalSection (&p_mutex->mutex);
+    p_mutex->locked = 0;
+    p_mutex->b_recursive = false;
     p_mutex->dynamic = true;
 }
 
 void vlc_mutex_init_recursive( vlc_mutex_t *p_mutex )
 {
     InitializeCriticalSection( &p_mutex->mutex );
+    p_mutex->locked = 0;
+    p_mutex->b_recursive = true;
     p_mutex->dynamic = true;
 }
 
@@ -126,12 +128,14 @@ void vlc_mutex_lock (vlc_mutex_t *p_mutex)
             SleepConditionVariableCS(&super_variable, &super_mutex, INFINITE);
             p_mutex->contention--;
         }
-        p_mutex->locked = true;
+        p_mutex->locked = 1;
         LeaveCriticalSection(&super_mutex);
         return;
     }
 
     EnterCriticalSection (&p_mutex->mutex);
+    assert(!p_mutex->locked || p_mutex->b_recursive);
+    p_mutex->locked++;
 }
 
 int vlc_mutex_trylock (vlc_mutex_t *p_mutex)
@@ -143,14 +147,26 @@ int vlc_mutex_trylock (vlc_mutex_t *p_mutex)
         EnterCriticalSection(&super_mutex);
         if (!p_mutex->locked)
         {
-            p_mutex->locked = true;
+            p_mutex->locked = 1;
             ret = 0;
         }
         LeaveCriticalSection(&super_mutex);
         return ret;
     }
 
-    return TryEnterCriticalSection (&p_mutex->mutex) ? 0 : EBUSY;
+    int ret = TryEnterCriticalSection (&p_mutex->mutex);
+    if (ret == 0)
+        return EBUSY;
+
+    assert(!p_mutex->locked || p_mutex->b_recursive);
+    if (unlikely(p_mutex->locked && !p_mutex->b_recursive))
+    {
+        LeaveCriticalSection (&p_mutex->mutex);
+        return EDEADLK; /* we already had the non-recursive lock */
+    }
+
+    p_mutex->locked++;
+    return 0;
 }
 
 void vlc_mutex_unlock (vlc_mutex_t *p_mutex)
@@ -159,13 +175,15 @@ void vlc_mutex_unlock (vlc_mutex_t *p_mutex)
     {   /* static mutexes */
         EnterCriticalSection(&super_mutex);
         assert (p_mutex->locked);
-        p_mutex->locked = false;
+        p_mutex->locked = 0;
         if (p_mutex->contention)
             WakeAllConditionVariable(&super_variable);
         LeaveCriticalSection(&super_mutex);
         return;
     }
 
+    p_mutex->locked--;
+    assert(p_mutex->locked >= 0);
     LeaveCriticalSection (&p_mutex->mutex);
 }
 
-- 
2.8.2