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

[Steve Lhomme]

locked & b_recursive are always accessed in the CriticalSection

--
replaces https://patches.videolan.org/patch/13622/ by reusing the locked value
it needs to count the number of locks so that recursive unlocks don't consider
it's unlocked the first time

trylock doesn't assert on EBUSY, just like with pthread

replaces https://patches.videolan.org/patch/13639/ by unlocking when trylock
returns EDEADLK
---
 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 963a8d7..55c2013 100644
--- a/src/win32/thread.c
+++ b/src/win32/thread.c
@@ -69,15 +69,17 @@ struct vlc_thread
 /*** 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;
 }
 
@@ -102,13 +104,15 @@ void vlc_mutex_lock (vlc_mutex_t *p_mutex)
             vlc_cond_wait (&super_variable, &super_mutex);
             p_mutex->contention--;
         }
-        p_mutex->locked = true;
+        p_mutex->locked = 1;
         vlc_mutex_unlock (&super_mutex);
         vlc_restorecancel (canc);
         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)
@@ -121,14 +125,26 @@ int vlc_mutex_trylock (vlc_mutex_t *p_mutex)
         vlc_mutex_lock (&super_mutex);
         if (!p_mutex->locked)
         {
-            p_mutex->locked = true;
+            p_mutex->locked = 1;
             ret = 0;
         }
         vlc_mutex_unlock (&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)
@@ -139,13 +155,15 @@ void vlc_mutex_unlock (vlc_mutex_t *p_mutex)
 
         vlc_mutex_lock (&super_mutex);
         assert (p_mutex->locked);
-        p_mutex->locked = false;
+        p_mutex->locked = 0;
         if (p_mutex->contention)
             vlc_cond_broadcast (&super_variable);
         vlc_mutex_unlock (&super_mutex);
         return;
     }
 
+    p_mutex->locked--;
+    assert(p_mutex->locked >= 0);
     LeaveCriticalSection (&p_mutex->mutex);
 }
 
-- 
2.7.0