[vlc-commits] thread: use the common vlc_cond_t implementation

Rémi Denis-Courmont git at videolan.org
Fri Feb 21 18:26:16 CET 2020


vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Wed Feb 19 21:46:09 2020 +0200| [ad6983eef48be550213e3b61f3b44ea1f43adf4a] | committer: Rémi Denis-Courmont

thread: use the common vlc_cond_t implementation

This gets rid of some platform-specific code. The more interesting
consequence is allowing custom mutex implementation later, which would
not otherwise be possible, due to vlc_cond_wait() entanglement.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=ad6983eef48be550213e3b61f3b44ea1f43adf4a
---

 include/vlc_threads.h |  59 ++++++-----------
 src/darwin/thread.c   | 108 -------------------------------
 src/misc/threads.c    |   9 ++-
 src/os2/thread.c      | 175 --------------------------------------------------
 src/posix/thread.c    |  61 ------------------
 5 files changed, 22 insertions(+), 390 deletions(-)

diff --git a/include/vlc_threads.h b/include/vlc_threads.h
index 2ffeeb0b2c..836f042891 100644
--- a/include/vlc_threads.h
+++ b/include/vlc_threads.h
@@ -77,7 +77,6 @@ typedef struct
     };
 } vlc_mutex_t;
 #define VLC_STATIC_MUTEX { false, { { false, 0 } } }
-#define LIBVLC_NEED_CONDVAR
 #define LIBVLC_NEED_RWLOCK
 typedef INIT_ONCE vlc_once_t;
 #define VLC_STATIC_ONCE INIT_ONCE_STATIC_INIT
@@ -126,14 +125,6 @@ typedef struct
     };
 } vlc_mutex_t;
 #define VLC_STATIC_MUTEX { false, { { false, 0 } } }
-typedef struct
-{
-    HEV      hev;
-    unsigned waiters;
-    HEV      hevAck;
-    unsigned signaled;
-} vlc_cond_t;
-#define VLC_STATIC_COND { NULLHANDLE, 0, NULLHANDLE, 0 }
 #define LIBVLC_NEED_RWLOCK
 typedef struct
 {
@@ -181,7 +172,6 @@ static inline int vlc_poll (struct pollfd *fds, unsigned nfds, int timeout)
 # include <poll.h>
 # define LIBVLC_USE_PTHREAD_CLEANUP   1
 # define LIBVLC_NEED_SLEEP
-# define LIBVLC_NEED_CONDVAR
 # define LIBVLC_NEED_RWLOCK
 
 typedef struct vlc_thread *vlc_thread_t;
@@ -237,8 +227,6 @@ typedef pthread_t       vlc_osthread_t;
 #define vlc_thread_equal(a,b) pthread_equal(a,b)
 typedef pthread_mutex_t vlc_mutex_t;
 #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER
-typedef pthread_cond_t vlc_cond_t;
-#define VLC_STATIC_COND PTHREAD_COND_INITIALIZER
 typedef pthread_rwlock_t vlc_rwlock_t;
 #define VLC_STATIC_RWLOCK PTHREAD_RWLOCK_INITIALIZER
 typedef pthread_once_t  vlc_once_t;
@@ -299,28 +287,6 @@ typedef pthread_mutex_t vlc_mutex_t;
  */
 #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER
 
-/**
- * Condition variable.
- *
- * Storage space for a thread condition variable.
- *
- * \ingroup condvar
- */
-typedef pthread_cond_t  vlc_cond_t;
-
-/**
- * Static initializer for (static) condition variable.
- *
- * \note
- * The condition variable will use the default clock, which is OS-dependent.
- * Therefore, where timed waits are necessary the condition variable should
- * always be initialized dynamically explicit instead of using this
- * initializer.
- *
- * \ingroup condvar
- */
-#define VLC_STATIC_COND  PTHREAD_COND_INITIALIZER
-
 /**
  * Read/write lock.
  *
@@ -472,16 +438,29 @@ VLC_API bool vlc_mutex_marked(const vlc_mutex_t *) VLC_USED;
  * @{
  */
 
-#ifdef LIBVLC_NEED_CONDVAR
 struct vlc_cond_waiter;
 
+/**
+ * Condition variable.
+ *
+ * Storage space for a thread condition variable.
+ */
 typedef struct
 {
     struct vlc_cond_waiter *head;
     vlc_mutex_t lock;
 } vlc_cond_t;
-# define VLC_STATIC_COND { NULL, VLC_STATIC_MUTEX }
-#endif
+
+/**
+ * Static initializer for (static) condition variable.
+ *
+ * \note
+ * The condition variable will use the default clock, which is OS-dependent.
+ * Therefore, where timed waits are necessary the condition variable should
+ * always be initialized dynamically explicit instead of using this
+ * initializer.
+ */
+#define VLC_STATIC_COND { NULL, VLC_STATIC_MUTEX }
 
 /**
  * Initializes a condition variable.
@@ -1197,12 +1176,10 @@ static inline void vlc_cleanup_lock (void *lock)
 }
 #define mutex_cleanup_push( lock ) vlc_cleanup_push (vlc_cleanup_lock, lock)
 
-#if defined(LIBVLC_NEED_CONDVAR) && !defined(__cplusplus)
+#ifndef __cplusplus
 void vlc_cancel_addr_set(atomic_uint *addr);
 void vlc_cancel_addr_clear(atomic_uint *addr);
-#endif
-
-#ifdef __cplusplus
+#else
 /**
  * Helper C++ class to lock a mutex.
  *
diff --git a/src/darwin/thread.c b/src/darwin/thread.c
index 349a4935f3..eb0a25fd8c 100644
--- a/src/darwin/thread.c
+++ b/src/darwin/thread.c
@@ -204,114 +204,6 @@ void vlc_mutex_unlock (vlc_mutex_t *p_mutex)
     vlc_mutex_unmark(p_mutex);
 }
 
-void vlc_cond_init (vlc_cond_t *p_condvar)
-{
-    if (unlikely(pthread_cond_init (p_condvar, NULL)))
-        abort ();
-}
-
-void vlc_cond_init_daytime (vlc_cond_t *p_condvar)
-{
-    if (unlikely(pthread_cond_init (p_condvar, NULL)))
-        abort ();
-}
-
-void vlc_cond_destroy (vlc_cond_t *p_condvar)
-{
-    int val = pthread_cond_destroy (p_condvar);
-
-    /* due to a faulty pthread implementation within Darwin 11 and
-     * later condition variables cannot be destroyed without
-     * terminating the application immediately.
-     * This Darwin kernel issue is still present in version 13
-     * and might not be resolved prior to Darwin 15.
-     * radar://12496249
-     *
-     * To work-around this, we are just leaking the condition variable
-     * which is acceptable due to VLC's low number of created variables
-     * and its usually limited runtime.
-     * Ideally, we should implement a re-useable pool.
-     */
-    if (val != 0) {
-        #ifndef NDEBUG
-        printf("pthread_cond_destroy returned %i\n", val);
-        #endif
-
-        if (val == EBUSY)
-            return;
-    }
-
-    VLC_THREAD_ASSERT ("destroying condition");
-}
-
-void vlc_cond_signal (vlc_cond_t *p_condvar)
-{
-    int val = pthread_cond_signal (p_condvar);
-    VLC_THREAD_ASSERT ("signaling condition variable");
-}
-
-void vlc_cond_broadcast (vlc_cond_t *p_condvar)
-{
-    pthread_cond_broadcast (p_condvar);
-}
-
-void vlc_cond_wait (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex)
-{
-    int val = pthread_cond_wait (p_condvar, p_mutex);
-    VLC_THREAD_ASSERT ("waiting on condition");
-}
-
-int vlc_cond_timedwait (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex,
-                        vlc_tick_t deadline)
-{
-    /* according to POSIX standards, cond_timedwait should be a cancellation point
-     * Of course, Darwin does not care */
-    pthread_testcancel();
-
-    /*
-     * vlc_tick_now() is the monotonic clock, pthread_cond_timedwait expects
-     * origin of gettimeofday(). Use timedwait_relative_np() instead.
-     */
-    vlc_tick_t base = vlc_tick_now();
-    deadline -= base;
-    if (deadline < 0)
-        deadline = 0;
-
-    struct timespec ts = timespec_from_vlc_tick(deadline);
-    int val = pthread_cond_timedwait_relative_np(p_condvar, p_mutex, &ts);
-    if (val != ETIMEDOUT)
-        VLC_THREAD_ASSERT ("timed-waiting on condition");
-    return val;
-}
-
-/* variant for vlc_cond_init_daytime */
-int vlc_cond_timedwait_daytime (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex,
-                                time_t deadline)
-{
-    /*
-     * Note that both pthread_cond_timedwait_relative_np and pthread_cond_timedwait
-     * convert the given timeout to a mach absolute deadline, with system startup
-     * as the time origin. There is no way you can change this behaviour.
-     *
-     * For more details, see: https://devforums.apple.com/message/931605
-     */
-
-    pthread_testcancel();
-
-    /*
-     * FIXME: It is assumed, that in this case the system waits until the real
-     * time deadline is passed, even if the real time is adjusted in between.
-     * This is not fulfilled, as described above.
-     */
-    struct timespec ts = { deadline, 0 };
-    int val = pthread_cond_timedwait(p_condvar, p_mutex, &ts);
-
-    if (val != ETIMEDOUT)
-        VLC_THREAD_ASSERT ("timed-waiting on condition");
-    return val;
-}
-
-
 void vlc_rwlock_init (vlc_rwlock_t *lock)
 {
     if (unlikely(pthread_rwlock_init (lock, NULL)))
diff --git a/src/misc/threads.c b/src/misc/threads.c
index a75ca50652..bdb3c9d687 100644
--- a/src/misc/threads.c
+++ b/src/misc/threads.c
@@ -155,9 +155,7 @@ bool vlc_mutex_marked(const vlc_mutex_t *mutex)
 # undef LIBVLC_NEED_SLEEP
 #endif
 
-#if defined(LIBVLC_NEED_SLEEP) || defined(LIBVLC_NEED_CONDVAR)
-#include <stdatomic.h>
-
+#if defined(_WIN32) || defined(__ANDROID__)
 static void do_vlc_cancel_addr_clear(void *addr)
 {
     vlc_cancel_addr_clear(addr);
@@ -179,6 +177,9 @@ static void vlc_cancel_addr_finish(atomic_uint *addr)
     /* Act on cancellation as potential wake-up source */
     vlc_testcancel();
 }
+#else
+# define vlc_cancel_addr_prepare(addr) ((void)0)
+# define vlc_cancel_addr_finish(addr) ((void)0)
 #endif
 
 #ifdef LIBVLC_NEED_SLEEP
@@ -200,7 +201,6 @@ void (vlc_tick_sleep)(vlc_tick_t delay)
 }
 #endif
 
-#ifdef LIBVLC_NEED_CONDVAR
 void vlc_cond_init(vlc_cond_t *cond)
 {
     cond->head = NULL;
@@ -371,7 +371,6 @@ int vlc_cond_timedwait_daytime(vlc_cond_t *cond, vlc_mutex_t *mutex,
 
     return ret;
 }
-#endif
 
 #ifdef LIBVLC_NEED_RWLOCK
 /*** Generic read/write locks ***/
diff --git a/src/os2/thread.c b/src/os2/thread.c
index 150b916988..6b04c47610 100644
--- a/src/os2/thread.c
+++ b/src/os2/thread.c
@@ -134,8 +134,6 @@ static vlc_mutex_t super_mutex;
 static vlc_cond_t  super_variable;
 extern vlc_rwlock_t config_lock;
 
-static void vlc_static_cond_destroy_all(void);
-
 int _CRT_init(void);
 void _CRT_term(void);
 
@@ -163,7 +161,6 @@ unsigned long _System _DLL_InitTerm(unsigned long hmod, unsigned long flag)
             vlc_threadvar_delete (&thread_key);
             vlc_cond_destroy (&super_variable);
             vlc_mutex_destroy (&super_mutex);
-            vlc_static_cond_destroy_all ();
 
             _CRT_term();
 
@@ -264,178 +261,6 @@ void vlc_mutex_unlock (vlc_mutex_t *p_mutex)
     vlc_mutex_unmark(p_mutex);
 }
 
-/*** Condition variables ***/
-typedef struct vlc_static_cond_t vlc_static_cond_t;
-
-struct vlc_static_cond_t
-{
-    vlc_cond_t condvar;
-    vlc_static_cond_t *next;
-};
-
-static vlc_static_cond_t *static_condvar_start = NULL;
-
-static void vlc_static_cond_init (vlc_cond_t *p_condvar)
-{
-    vlc_mutex_lock (&super_mutex);
-
-    if (p_condvar->hev == NULLHANDLE)
-    {
-        vlc_cond_init (p_condvar);
-
-        vlc_static_cond_t *new_static_condvar;
-
-        new_static_condvar = malloc (sizeof (*new_static_condvar));
-        if (unlikely (!new_static_condvar))
-            abort();
-
-        memcpy (&new_static_condvar->condvar, p_condvar, sizeof (*p_condvar));
-        new_static_condvar->next = static_condvar_start;
-        static_condvar_start = new_static_condvar;
-    }
-
-    vlc_mutex_unlock (&super_mutex);
-}
-
-static void vlc_static_cond_destroy_all (void)
-{
-    vlc_static_cond_t *static_condvar;
-    vlc_static_cond_t *static_condvar_next;
-
-
-    for (static_condvar = static_condvar_start; static_condvar;
-         static_condvar = static_condvar_next)
-    {
-        static_condvar_next = static_condvar->next;
-
-        vlc_cond_destroy (&static_condvar->condvar);
-        free (static_condvar);
-    }
-}
-
-void vlc_cond_init (vlc_cond_t *p_condvar)
-{
-    if (DosCreateEventSem (NULL, &p_condvar->hev, 0, FALSE) ||
-        DosCreateEventSem (NULL, &p_condvar->hevAck, 0, FALSE))
-        abort();
-
-    p_condvar->waiters = 0;
-    p_condvar->signaled = 0;
-}
-
-void vlc_cond_init_daytime (vlc_cond_t *p_condvar)
-{
-    vlc_cond_init (p_condvar);
-}
-
-void vlc_cond_destroy (vlc_cond_t *p_condvar)
-{
-    DosCloseEventSem( p_condvar->hev );
-    DosCloseEventSem( p_condvar->hevAck );
-}
-
-void vlc_cond_signal (vlc_cond_t *p_condvar)
-{
-    if (p_condvar->hev == NULLHANDLE)
-        vlc_static_cond_init (p_condvar);
-
-    if (!__atomic_cmpxchg32 (&p_condvar->waiters, 0, 0))
-    {
-        ULONG ulPost;
-
-        __atomic_xchg (&p_condvar->signaled, 1);
-        DosPostEventSem (p_condvar->hev);
-
-        DosWaitEventSem (p_condvar->hevAck, SEM_INDEFINITE_WAIT);
-        DosResetEventSem (p_condvar->hevAck, &ulPost);
-    }
-}
-
-void vlc_cond_broadcast (vlc_cond_t *p_condvar)
-{
-    if (p_condvar->hev == NULLHANDLE)
-        vlc_static_cond_init (p_condvar);
-
-    while (!__atomic_cmpxchg32 (&p_condvar->waiters, 0, 0))
-        vlc_cond_signal (p_condvar);
-}
-
-static int vlc_cond_wait_common (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex,
-                                 ULONG ulTimeout)
-{
-    ULONG ulPost;
-    ULONG rc;
-
-    assert(p_condvar->hev != NULLHANDLE);
-
-    do
-    {
-        vlc_testcancel();
-
-        __atomic_increment (&p_condvar->waiters);
-
-        vlc_mutex_unlock (p_mutex);
-
-        do
-        {
-            rc = vlc_WaitForSingleObject( p_condvar->hev, ulTimeout );
-            if (rc == NO_ERROR)
-                DosResetEventSem (p_condvar->hev, &ulPost);
-        } while (rc == NO_ERROR &&
-                 __atomic_cmpxchg32 (&p_condvar->signaled, 0, 1) == 0);
-
-        __atomic_decrement (&p_condvar->waiters);
-
-        DosPostEventSem (p_condvar->hevAck);
-
-        vlc_mutex_lock (p_mutex);
-    } while( rc == ERROR_INTERRUPT );
-
-    return rc ? ETIMEDOUT : 0;
-}
-
-void vlc_cond_wait (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex)
-{
-    if (p_condvar->hev == NULLHANDLE)
-        vlc_static_cond_init (p_condvar);
-
-    vlc_cond_wait_common (p_condvar, p_mutex, SEM_INDEFINITE_WAIT);
-}
-
-int vlc_cond_timedwait (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex,
-                        vlc_tick_t deadline)
-{
-    ULONG ulTimeout;
-
-    vlc_tick_t total = vlc_tick_now();
-    total = (deadline - total) / 1000;
-    if( total < 0 )
-        total = 0;
-
-    ulTimeout = ( total > 0x7fffffff ) ? 0x7fffffff : total;
-
-    return vlc_cond_wait_common (p_condvar, p_mutex, ulTimeout);
-}
-
-int vlc_cond_timedwait_daytime (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex,
-                                time_t deadline)
-{
-    ULONG ulTimeout;
-    vlc_tick_t total;
-    struct timeval tv;
-
-    gettimeofday (&tv, NULL);
-
-    total = vlc_tick_from_timeval( &tv );
-    total = (deadline - total) / 1000;
-    if( total < 0 )
-        total = 0;
-
-    ulTimeout = ( total > 0x7fffffff ) ? 0x7fffffff : total;
-
-    return vlc_cond_wait_common (p_condvar, p_mutex, ulTimeout);
-}
-
 void vlc_once(vlc_once_t *once, void (*cb)(void))
 {
     unsigned done;
diff --git a/src/posix/thread.c b/src/posix/thread.c
index 4695056d07..1756e43800 100644
--- a/src/posix/thread.c
+++ b/src/posix/thread.c
@@ -165,67 +165,6 @@ void vlc_mutex_unlock(vlc_mutex_t *mutex)
     vlc_mutex_unmark(mutex);
 }
 
-void vlc_cond_init (vlc_cond_t *p_condvar)
-{
-    pthread_condattr_t attr;
-
-    if (unlikely(pthread_condattr_init (&attr))
-     || unlikely(pthread_condattr_setclock(&attr, CLOCK_MONOTONIC))
-     || unlikely(pthread_cond_init (p_condvar, &attr)))
-        abort ();
-
-    pthread_condattr_destroy (&attr);
-}
-
-void vlc_cond_init_daytime (vlc_cond_t *p_condvar)
-{
-    if (unlikely(pthread_cond_init (p_condvar, NULL)))
-        abort ();
-}
-
-void vlc_cond_destroy (vlc_cond_t *p_condvar)
-{
-    int val = pthread_cond_destroy( p_condvar );
-    VLC_THREAD_ASSERT ("destroying condition");
-}
-
-void vlc_cond_signal (vlc_cond_t *p_condvar)
-{
-    int val = pthread_cond_signal( p_condvar );
-    VLC_THREAD_ASSERT ("signaling condition variable");
-}
-
-void vlc_cond_broadcast (vlc_cond_t *p_condvar)
-{
-    pthread_cond_broadcast (p_condvar);
-}
-
-void vlc_cond_wait (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex)
-{
-    int val = pthread_cond_wait( p_condvar, p_mutex );
-    VLC_THREAD_ASSERT ("waiting on condition");
-}
-
-int vlc_cond_timedwait (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex,
-                        vlc_tick_t deadline)
-{
-    struct timespec ts = timespec_from_vlc_tick (deadline);
-    int val = pthread_cond_timedwait (p_condvar, p_mutex, &ts);
-    if (val != ETIMEDOUT)
-        VLC_THREAD_ASSERT ("timed-waiting on condition");
-    return val;
-}
-
-int vlc_cond_timedwait_daytime (vlc_cond_t *p_condvar, vlc_mutex_t *p_mutex,
-                                time_t deadline)
-{
-    struct timespec ts = { deadline, 0 };
-    int val = pthread_cond_timedwait (p_condvar, p_mutex, &ts);
-    if (val != ETIMEDOUT)
-        VLC_THREAD_ASSERT ("timed-waiting on condition");
-    return val;
-}
-
 void vlc_rwlock_init (vlc_rwlock_t *lock)
 {
     if (unlikely(pthread_rwlock_init (lock, NULL)))



More information about the vlc-commits mailing list