[vlc-devel] [PATCH 4/6] thread: remove the mutex mark system

RĂ©mi Denis-Courmont remi at remlab.net
Thu Feb 20 20:58:21 CET 2020


This is now redundant.
---
 include/vlc_threads.h |  13 +++---
 src/libvlc.h          |  15 -------
 src/libvlccore.sym    |   2 +-
 src/misc/threads.c    | 100 +-----------------------------------------
 src/player/player.c   |   6 +--
 5 files changed, 11 insertions(+), 125 deletions(-)

diff --git a/include/vlc_threads.h b/include/vlc_threads.h
index 0aa1dbca05..02060cc775 100644
--- a/include/vlc_threads.h
+++ b/include/vlc_threads.h
@@ -400,18 +400,17 @@ VLC_API void vlc_mutex_unlock(vlc_mutex_t *);
  * This function has no effects.
  * It is only meant to be use in run-time assertions.
  *
- * @retval false in debug builds of LibVLC,
- *               if the mutex is not locked by the calling thread;
- * @retval true in debug builds of LibVLC,
- *              if the mutex is locked by the calling thread;
- * @retval true in release builds of LibVLC.
+ * @warning This function might not return correct results in non-debug builds.
+ *
+ * @retval false the mutex is not locked by the calling thread
+ * @retval true the mutex is locked by the calling thread
  */
-VLC_API bool vlc_mutex_marked(const vlc_mutex_t *) VLC_USED;
+VLC_API bool vlc_mutex_held(const vlc_mutex_t *) VLC_USED;
 
 /**
  * Asserts that a mutex is locked by the calling thread.
  */
-#define vlc_mutex_assert(m) assert(vlc_mutex_marked(m))
+#define vlc_mutex_assert(m) assert(vlc_mutex_held(m))
 
 /** @} */
 
diff --git a/src/libvlc.h b/src/libvlc.h
index ff50bd9218..e91587f947 100644
--- a/src/libvlc.h
+++ b/src/libvlc.h
@@ -54,21 +54,6 @@ void vlc_threads_setup (libvlc_int_t *);
 void vlc_trace (const char *fn, const char *file, unsigned line);
 #define vlc_backtrace() vlc_trace(__func__, __FILE__, __LINE__)
 
-#ifndef NDEBUG
-/**
- * Marks a mutex locked.
- */
-void vlc_mutex_mark(const vlc_mutex_t *);
-
-/**
- * Unmarks a mutex.
- */
-void vlc_mutex_unmark(const vlc_mutex_t *);
-#else
-# define vlc_mutex_mark(m) ((void)(m))
-# define vlc_mutex_unmark(m) ((void)(m))
-#endif
-
 /*
  * Logging
  */
diff --git a/src/libvlccore.sym b/src/libvlccore.sym
index 1d9e521662..a0efbecb78 100644
--- a/src/libvlccore.sym
+++ b/src/libvlccore.sym
@@ -619,7 +619,7 @@ vlc_mutex_init_recursive
 vlc_mutex_lock
 vlc_mutex_trylock
 vlc_mutex_unlock
-vlc_mutex_marked
+vlc_mutex_held
 vlc_global_mutex
 vlc_object_create
 vlc_object_delete
diff --git a/src/misc/threads.c b/src/misc/threads.c
index eddf06d348..c4fd617882 100644
--- a/src/misc/threads.c
+++ b/src/misc/threads.c
@@ -62,99 +62,6 @@ void vlc_global_mutex (unsigned n, bool acquire)
         vlc_mutex_unlock (lock);
 }
 
-#ifndef NDEBUG
-# ifdef HAVE_SEARCH_H
-#  include <search.h>
-# endif
-
-struct vlc_lock_mark
-{
-    const void *object;
-    uintptr_t refs;
-};
-
-static int vlc_lock_mark_cmp(const void *a, const void *b)
-{
-    const struct vlc_lock_mark *ma = a, *mb = b;
-
-    if (ma->object == mb->object)
-        return 0;
-
-    return ((uintptr_t)(ma->object) > (uintptr_t)(mb->object)) ? +1 : -1;
-}
-
-static void vlc_lock_mark(const void *lock, void **rootp)
-{
-    struct vlc_lock_mark *mark = malloc(sizeof (*mark));
-    if (unlikely(mark == NULL))
-        abort();
-
-    mark->object = lock;
-    mark->refs = 0;
-
-    void **entry = tsearch(mark, rootp, vlc_lock_mark_cmp);
-    if (unlikely(entry == NULL))
-        abort();
-
-    if (unlikely(*entry != mark)) {
-        /* Recursive locking: lock is already in the tree */
-        free(mark);
-        mark = *entry;
-    }
-
-    mark->refs++;
-}
-
-static void vlc_lock_unmark(const void *lock, void **rootp)
-{
-    struct vlc_lock_mark *mark = &(struct vlc_lock_mark){ lock, 0 };
-    void **entry = tfind(mark, rootp, vlc_lock_mark_cmp);
-
-    assert(entry != NULL);
-    mark = *entry;
-    assert(mark->refs > 0);
-
-    if (likely(--mark->refs == 0)) {
-        tdelete(mark, rootp, vlc_lock_mark_cmp);
-        free(mark);
-    }
-}
-
-static bool vlc_lock_marked(const void *lock, void **rootp)
-{
-    struct vlc_lock_mark *mark = &(struct vlc_lock_mark){ lock, 0 };
-
-    return tfind(mark, rootp, vlc_lock_mark_cmp) != NULL;
-}
-
-static _Thread_local void *vlc_mutex_marks = NULL;
-
-void vlc_mutex_mark(const vlc_mutex_t *mutex)
-{
-    vlc_lock_mark(mutex, &vlc_mutex_marks);
-}
-
-void vlc_mutex_unmark(const vlc_mutex_t *mutex)
-{
-    vlc_lock_unmark(mutex, &vlc_mutex_marks);
-}
-
-bool vlc_mutex_marked(const vlc_mutex_t *mutex)
-{
-    return vlc_lock_marked(mutex, &vlc_mutex_marks);
-}
-#else
-bool vlc_mutex_marked(const vlc_mutex_t *mutex)
-{
-    return true;
-}
-#endif
-
-#if defined (_WIN32) && (_WIN32_WINNT < _WIN32_WINNT_WIN8)
-/* Cannot define OS version-dependent stuff in public headers */
-# undef LIBVLC_NEED_SLEEP
-#endif
-
 #if defined(_WIN32) || defined(__ANDROID__)
 static void do_vlc_cancel_addr_clear(void *addr)
 {
@@ -229,7 +136,7 @@ void vlc_mutex_destroy(vlc_mutex_t *mtx)
 static _Thread_local char thread_self[1];
 #define THREAD_SELF ((const void *)thread_self)
 
-static bool vlc_mutex_held(const vlc_mutex_t *mtx)
+bool vlc_mutex_held(const vlc_mutex_t *mtx)
 {
     /* This comparison is thread-safe:
      * Even though other threads may modify the owner field at any time,
@@ -260,7 +167,6 @@ void vlc_mutex_lock(vlc_mutex_t *mtx)
 
     vlc_restorecancel(canc);
     atomic_store_explicit(&mtx->owner, THREAD_SELF, memory_order_relaxed);
-    vlc_mutex_mark(mtx);
 }
 
 int vlc_mutex_trylock(vlc_mutex_t *mtx)
@@ -278,7 +184,6 @@ int vlc_mutex_trylock(vlc_mutex_t *mtx)
          */
         atomic_store_explicit(&mtx->recursion, recursion + 1,
                               memory_order_relaxed);
-        vlc_mutex_mark(mtx);
         return 0;
     } else
         assert(!vlc_mutex_held(mtx));
@@ -289,7 +194,6 @@ int vlc_mutex_trylock(vlc_mutex_t *mtx)
                                                 memory_order_acquire,
                                                 memory_order_relaxed)) {
         atomic_store_explicit(&mtx->owner, THREAD_SELF, memory_order_relaxed);
-        vlc_mutex_mark(mtx);
         return 0;
     }
 
@@ -306,12 +210,10 @@ void vlc_mutex_unlock(vlc_mutex_t *mtx)
         /* Non-last recursive unlocking. */
         atomic_store_explicit(&mtx->recursion, recursion - 1,
                               memory_order_relaxed);
-        vlc_mutex_unmark(mtx);
         return;
     }
 
     atomic_store_explicit(&mtx->owner, NULL, memory_order_relaxed);
-    vlc_mutex_unmark(mtx);
 
     switch (atomic_exchange_explicit(&mtx->value, 0, memory_order_release)) {
         case 2:
diff --git a/src/player/player.c b/src/player/player.c
index 89c16caf6a..48c03905a4 100644
--- a/src/player/player.c
+++ b/src/player/player.c
@@ -914,11 +914,11 @@ vlc_player_Lock(vlc_player_t *player)
 {
     /* Vout and aout locks should not be held, cf. vlc_player_vout_cbs and
      * vlc_player_aout_cbs documentation */
-    assert(!vlc_mutex_marked(&player->vout_listeners_lock));
-    assert(!vlc_mutex_marked(&player->aout_listeners_lock));
+    assert(!vlc_mutex_held(&player->vout_listeners_lock));
+    assert(!vlc_mutex_held(&player->aout_listeners_lock));
     /* The timer lock should not be held (possible lock-order-inversion), cf.
      * vlc_player_timer_cbs.on_update documentation */
-    assert(!vlc_mutex_marked(&player->timer.lock));
+    assert(!vlc_mutex_held(&player->timer.lock));
 
     vlc_mutex_lock(&player->lock);
 }
-- 
2.25.0



More information about the vlc-devel mailing list