[vlc-devel] [PATCH 1/3] thread: rationalise vlc_atomic_timedwait()
Rémi Denis-Courmont
remi at remlab.net
Mon Feb 17 21:54:07 CET 2020
This changes the function prototype and semantics to more closely match
the other VLC timed-wait functions:
- Return 0 on success, or an error code on "error", i.e., time-out.
- Take an absolute deadline rather than a relative delay in.
This fixes time drift due to preemption and simplifies the call sites.
---
include/vlc_threads.h | 10 +++++-----
src/linux/thread.c | 26 +++++++++++++++++++++-----
src/misc/threads.c | 39 +++++++++++++--------------------------
src/posix/wait.c | 9 +++++----
src/win32/thread.c | 24 ++++++++++++++++++------
5 files changed, 62 insertions(+), 46 deletions(-)
diff --git a/include/vlc_threads.h b/include/vlc_threads.h
index ab9f25cb69..f52e572e6e 100644
--- a/include/vlc_threads.h
+++ b/include/vlc_threads.h
@@ -714,17 +714,17 @@ void vlc_atomic_wait(void *addr, unsigned val);
* Waits on an address with a time-out.
*
* This function operates as vlc_atomic_wait() but provides an additional
- * time-out. If the time-out elapses, the thread resumes and the function
+ * time-out. If the deadline is reached, the thread resumes and the function
* returns.
*
* \param addr address to check for
* \param val value to match at the address
- * \param delay time-out duration
+ * \param deadline deadline to wait until
*
- * \retval true the function was woken up before the time-out
- * \retval false the time-out elapsed
+ * \retval 0 the function was woken up before the time-out
+ * \retval ETIMEDOUT the deadline was reached
*/
-bool vlc_atomic_timedwait(void *addr, unsigned val, vlc_tick_t delay);
+int vlc_atomic_timedwait(void *addr, unsigned val, vlc_tick_t deadline);
/**
* Wakes up one thread on an address.
diff --git a/src/linux/thread.c b/src/linux/thread.c
index b926f4e1b3..bf5f665003 100644
--- a/src/linux/thread.c
+++ b/src/linux/thread.c
@@ -22,6 +22,7 @@
# include "config.h"
#endif
+#include <assert.h>
#include <errno.h>
#include <limits.h>
#include <stdlib.h>
@@ -33,6 +34,7 @@
#ifndef FUTEX_PRIVATE_FLAG
#define FUTEX_WAKE_PRIVATE FUTEX_WAKE
#define FUTEX_WAIT_PRIVATE FUTEX_WAIT
+#define FUTEX_WAIT_BITSET_PRIVATE FUTEX_WAIT_BITSET
#endif
#include <vlc_common.h>
@@ -66,7 +68,8 @@ static int vlc_futex_wait(void *addr, unsigned val, const struct timespec *to)
int type;
pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &type);
#endif
- ret = sys_futex(addr, FUTEX_WAIT_PRIVATE, val, to, NULL, 0);
+ ret = sys_futex(addr, FUTEX_WAIT_BITSET_PRIVATE, val, to, NULL,
+ FUTEX_BITSET_MATCH_ANY);
#ifndef __ANDROID__
pthread_setcanceltype(type, NULL);
#endif
@@ -88,9 +91,22 @@ void vlc_atomic_wait(void *addr, unsigned val)
vlc_futex_wait(addr, val, NULL);
}
-bool vlc_atomic_timedwait(void *addr, unsigned val, vlc_tick_t delay)
+int vlc_atomic_timedwait(void *addr, unsigned val, vlc_tick_t deadline)
{
- struct timespec ts = timespec_from_vlc_tick(delay);
-
- return (vlc_futex_wait(addr, val, &ts) == 0 || errno != ETIMEDOUT);
+ struct timespec ts = timespec_from_vlc_tick(deadline);
+
+ if (vlc_futex_wait(addr, val, &ts) == 0)
+ return 0;
+
+ switch (errno) {
+ case EINTR:
+ case EAGAIN:
+ return 0;
+ case EFAULT:
+ case EINVAL:
+ vlc_assert_unreachable(); /* BUG! */
+ default:
+ break;
+ }
+ return errno;
}
diff --git a/src/misc/threads.c b/src/misc/threads.c
index 9dc5c56186..3c21a8b36b 100644
--- a/src/misc/threads.c
+++ b/src/misc/threads.c
@@ -184,16 +184,12 @@ static void vlc_cancel_addr_finish(atomic_uint *addr)
#ifdef LIBVLC_NEED_SLEEP
void (vlc_tick_wait)(vlc_tick_t deadline)
{
- vlc_tick_t delay;
atomic_uint value = ATOMIC_VAR_INIT(0);
vlc_cancel_addr_prepare(&value);
- while ((delay = (deadline - vlc_tick_now())) > 0)
- {
- vlc_atomic_timedwait(&value, 0, delay);
+ while (vlc_atomic_timedwait(&value, 0, deadline) == 0)
vlc_testcancel();
- }
vlc_cancel_addr_finish(&value);
}
@@ -269,11 +265,13 @@ void vlc_cond_wait(vlc_cond_t *cond, vlc_mutex_t *mutex)
vlc_cancel_addr_finish(&cond->value);
}
-static int vlc_cond_wait_delay(vlc_cond_t *cond, vlc_mutex_t *mutex,
- vlc_tick_t delay)
+int vlc_cond_timedwait(vlc_cond_t *cond, vlc_mutex_t *mutex,
+ vlc_tick_t deadline)
{
unsigned value = atomic_load_explicit(&cond->value,
memory_order_relaxed);
+ int ret;
+
while (value & 1)
{
if (atomic_compare_exchange_weak_explicit(&cond->value, &value,
@@ -286,20 +284,12 @@ static int vlc_cond_wait_delay(vlc_cond_t *cond, vlc_mutex_t *mutex,
vlc_cancel_addr_prepare(&cond->value);
vlc_mutex_unlock(mutex);
- if (delay > 0)
- value = vlc_atomic_timedwait(&cond->value, value, delay);
- else
- value = 0;
+ ret = vlc_atomic_timedwait(&cond->value, value, deadline);
vlc_mutex_lock(mutex);
vlc_cancel_addr_finish(&cond->value);
- return value ? 0 : ETIMEDOUT;
-}
-
-int vlc_cond_timedwait(vlc_cond_t *cond, vlc_mutex_t *mutex, vlc_tick_t deadline)
-{
- return vlc_cond_wait_delay(cond, mutex, deadline - vlc_tick_now());
+ return ret;
}
int vlc_cond_timedwait_daytime(vlc_cond_t *cond, vlc_mutex_t *mutex,
@@ -309,9 +299,10 @@ int vlc_cond_timedwait_daytime(vlc_cond_t *cond, vlc_mutex_t *mutex,
vlc_tick_t deadline = vlc_tick_from_sec( deadline_daytime );
timespec_get(&ts, TIME_UTC);
- deadline -= vlc_tick_from_timespec( &ts );
+ /* real-time to monotonic timestamp conversion */
+ deadline += vlc_tick_from_timespec(&ts) - vlc_tick_now();
- return vlc_cond_wait_delay(cond, mutex, deadline);
+ return vlc_cond_timedwait(cond, mutex, deadline);
}
#endif
@@ -445,13 +436,9 @@ int vlc_sem_timedwait(vlc_sem_t *sem, vlc_tick_t deadline)
{
if (likely(exp == 0))
{
- vlc_tick_t delay = deadline - vlc_tick_now();
-
- if (delay < 0)
- delay = 0;
-
- if (!vlc_atomic_timedwait(&sem->value, 0, delay))
- return ETIMEDOUT;
+ int ret = vlc_atomic_timedwait(&sem->value, 0, deadline);
+ if (ret)
+ return ret;
exp = 1;
}
diff --git a/src/posix/wait.c b/src/posix/wait.c
index 68b830b615..8e3156f514 100644
--- a/src/posix/wait.c
+++ b/src/posix/wait.c
@@ -85,15 +85,16 @@ void vlc_atomic_wait(void *addr, unsigned value)
pthread_cleanup_pop(1);
}
-bool vlc_atomic_timedwait(void *addr, unsigned value, vlc_tick_t delay)
+int vlc_atomic_timedwait(void *addr, unsigned value, vlc_tick_t deadline)
{
atomic_uint *futex = addr;
struct wait_bucket *bucket;
struct timespec ts;
- lldiv_t d = lldiv(delay, CLOCK_FREQ);
+ vlc_tick_t delay = deadline - vlc_tick_now();
+ lldiv_t d = lldiv((delay >= 0) ? delay : 0, CLOCK_FREQ);
int ret = 0;
- /* TODO: monotonic clock */
+ /* TODO: use monotonic clock directly */
clock_gettime(CLOCK_REALTIME, &ts);
ts.tv_sec += d.quot;
ts.tv_nsec += NS_FROM_VLC_TICK(d.rem);
@@ -110,7 +111,7 @@ bool vlc_atomic_timedwait(void *addr, unsigned value, vlc_tick_t delay)
ret = pthread_cond_timedwait(&bucket->wait, &bucket->lock, &ts);
pthread_cleanup_pop(1);
- return ret == 0;
+ return ret;
}
void vlc_atomic_notify_one(void *addr)
diff --git a/src/win32/thread.c b/src/win32/thread.c
index 4d6d7bdb5d..9f2c5e3f50 100644
--- a/src/win32/thread.c
+++ b/src/win32/thread.c
@@ -378,17 +378,29 @@ void vlc_atomic_wait(void *addr, unsigned val)
WaitOnAddress(addr, &val, sizeof (val), -1);
}
-bool vlc_atomic_timedwait(void *addr, unsigned val, vlc_tick_t delay)
+int vlc_atomic_timedwait(void *addr, unsigned val, vlc_tick_t deadline)
{
- delay = (delay + (1000-1)) / 1000;
+ vlc_tick_t delay;
- if (delay > 0x7fffffff)
+ do
{
- WaitOnAddress(addr, &val, sizeof (val), 0x7fffffff);
- return true; /* woke up early, claim spurious wake-up */
+ long ms;
+
+ delay = deadline - vlc_tick_now();
+
+ if (delay < 0)
+ ms = 0;
+ else if (delay >= VLC_TICK_FROM_MS(LONG_MAX))
+ ms = LONG_MAX;
+ else
+ ms = MS_FROM_VLC_TICK(delay);
+
+ if (WaitOnAddress(addr, &val, sizeof (val), ms))
+ return 0;
}
+ while (delay > 0);
- return WaitOnAddress(addr, &val, sizeof (val), delay);
+ return ETIMEDOUT;
}
void vlc_atomic_notify_one(void *addr)
--
2.25.0
More information about the vlc-devel
mailing list