test: make test-condvar call uv_cond_wait
Problem: The condvar tests could pass without uv_cond_wait (or uv_cond_timedwait) ever being invoked. Solution: Introduce semaphores to enforce ordering. Now there will always be a thread waiting on the condition when a signal() occurs. Gotchas: 1. On Windows, waiting for a timeout may return earlier than requested, depending on the granularity of timer ticks. 2. Timeout bounds are tuned based on our CI machines. Bonuses: 1. I added additional test cases to complete the test matrix. 2. It seemed to me that several of the condvar tests were redundant, because they used timing to probabilistically explore cases where there would be "missed connections" (signal without a waiter). Because it was not clear to me what the purpose of such tests were, I have removed them. Fixes: https://github.com/libuv/libuv/issues/1714 PR-URL: https://github.com/libuv/libuv/pull/1718 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
This commit is contained in:
Jamie Davis
Santiago Gimeno
parent
5124b27d35
commit
f401e67b60
@ -25,221 +25,243 @@
|
||||
#include <string.h>
|
||||
#include <errno.h>
|
||||
|
||||
struct worker_config;
|
||||
|
||||
typedef void (*signal_func)(struct worker_config* c, int* flag);
|
||||
typedef int (*wait_func)(struct worker_config* c, const int* flag);
|
||||
|
||||
typedef struct worker_config {
|
||||
uv_sem_t sem_waiting; /* post before waiting. */
|
||||
uv_sem_t sem_signaled; /* post after signaling. */
|
||||
uv_mutex_t mutex;
|
||||
uv_cond_t cond;
|
||||
int signal_delay;
|
||||
int wait_delay;
|
||||
int use_broadcast;
|
||||
volatile int posted_1;
|
||||
volatile int posted_2;
|
||||
void (*signal_cond)(struct worker_config* c, volatile int* flag);
|
||||
int (*wait_cond)(struct worker_config* c, const volatile int* flag);
|
||||
int posted_1;
|
||||
int posted_2;
|
||||
signal_func signal_cond;
|
||||
wait_func wait_cond;
|
||||
} worker_config;
|
||||
|
||||
void worker_config_init(worker_config* wc,
|
||||
int use_broadcast,
|
||||
signal_func signal_f,
|
||||
wait_func wait_f) {
|
||||
/* Wipe. */
|
||||
memset(wc, 0, sizeof(*wc));
|
||||
|
||||
/* Copy vars. */
|
||||
wc->signal_cond = signal_f;
|
||||
wc->wait_cond = wait_f;
|
||||
wc->use_broadcast = use_broadcast;
|
||||
|
||||
/* Init. */
|
||||
ASSERT(0 == uv_sem_init(&wc->sem_waiting, 0));
|
||||
ASSERT(0 == uv_sem_init(&wc->sem_signaled, 0));
|
||||
ASSERT(0 == uv_cond_init(&wc->cond));
|
||||
ASSERT(0 == uv_mutex_init(&wc->mutex));
|
||||
}
|
||||
|
||||
void worker_config_destroy(worker_config* wc) {
|
||||
uv_mutex_destroy(&wc->mutex);
|
||||
uv_cond_destroy(&wc->cond);
|
||||
uv_sem_destroy(&wc->sem_signaled);
|
||||
uv_sem_destroy(&wc->sem_waiting);
|
||||
}
|
||||
|
||||
/* arg is a worker_config.
|
||||
* Call signal_cond then wait_cond.
|
||||
* Partner should call wait then signal. */
|
||||
static void worker(void* arg) {
|
||||
worker_config* c = arg;
|
||||
c->signal_cond(c, &c->posted_1);
|
||||
c->wait_cond(c, &c->posted_2);
|
||||
}
|
||||
|
||||
static void noop_worker(void* arg) {
|
||||
return;
|
||||
}
|
||||
|
||||
static void condvar_signal(worker_config* c, volatile int* flag) {
|
||||
if (c->signal_delay)
|
||||
uv_sleep(c->signal_delay);
|
||||
/* 1. Signal a waiting waiter.
|
||||
* 2. Tell waiter we finished. */
|
||||
static void condvar_signal(worker_config* c, int* flag) {
|
||||
/* Wait until waiter holds mutex and is preparing to wait. */
|
||||
uv_sem_wait(&c->sem_waiting);
|
||||
|
||||
/* Make sure waiter has begun waiting. */
|
||||
uv_mutex_lock(&c->mutex);
|
||||
|
||||
/* Help waiter differentiate between spurious and legitimate wakeup. */
|
||||
ASSERT(*flag == 0);
|
||||
*flag = 1;
|
||||
|
||||
if (c->use_broadcast)
|
||||
uv_cond_broadcast(&c->cond);
|
||||
else
|
||||
uv_cond_signal(&c->cond);
|
||||
|
||||
uv_mutex_unlock(&c->mutex);
|
||||
|
||||
/* Done signaling. */
|
||||
uv_sem_post(&c->sem_signaled);
|
||||
}
|
||||
|
||||
|
||||
static int condvar_wait(worker_config* c, const volatile int* flag) {
|
||||
/* 1. Wait on a signal.
|
||||
* 2. Ensure that the signaler finished. */
|
||||
static int condvar_wait(worker_config* c, const int* flag) {
|
||||
uv_mutex_lock(&c->mutex);
|
||||
if (c->wait_delay)
|
||||
uv_sleep(c->wait_delay);
|
||||
while (*flag == 0) {
|
||||
|
||||
/* Tell signal'er that I am waiting. */
|
||||
uv_sem_post(&c->sem_waiting);
|
||||
|
||||
/* Wait until I get a non-spurious signal. */
|
||||
do {
|
||||
uv_cond_wait(&c->cond, &c->mutex);
|
||||
}
|
||||
} while (*flag == 0);
|
||||
ASSERT(*flag == 1);
|
||||
|
||||
uv_mutex_unlock(&c->mutex);
|
||||
|
||||
/* Wait for my signal'er to finish. */
|
||||
uv_sem_wait(&c->sem_signaled);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
/* uv_cond_wait: One thread signals, the other waits. */
|
||||
TEST_IMPL(condvar_1) {
|
||||
uv_thread_t thread;
|
||||
worker_config wc;
|
||||
uv_thread_t thread;
|
||||
|
||||
memset(&wc, 0, sizeof(wc));
|
||||
wc.wait_delay = 100;
|
||||
wc.signal_cond = condvar_signal;
|
||||
wc.wait_cond = condvar_wait;
|
||||
|
||||
ASSERT(0 == uv_cond_init(&wc.cond));
|
||||
ASSERT(0 == uv_mutex_init(&wc.mutex));
|
||||
/* Helper signal-then-wait. */
|
||||
worker_config_init(&wc, 0, condvar_signal, condvar_wait);
|
||||
ASSERT(0 == uv_thread_create(&thread, worker, &wc));
|
||||
|
||||
/* We wait-then-signal. */
|
||||
ASSERT(0 == wc.wait_cond(&wc, &wc.posted_1));
|
||||
wc.signal_cond(&wc, &wc.posted_2);
|
||||
|
||||
ASSERT(0 == uv_thread_join(&thread));
|
||||
uv_mutex_destroy(&wc.mutex);
|
||||
uv_cond_destroy(&wc.cond);
|
||||
worker_config_destroy(&wc);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
/* uv_cond_wait: One thread broadcasts, the other waits. */
|
||||
TEST_IMPL(condvar_2) {
|
||||
uv_thread_t thread;
|
||||
worker_config wc;
|
||||
uv_thread_t thread;
|
||||
|
||||
memset(&wc, 0, sizeof(wc));
|
||||
wc.signal_delay = 100;
|
||||
wc.signal_cond = condvar_signal;
|
||||
wc.wait_cond = condvar_wait;
|
||||
|
||||
ASSERT(0 == uv_cond_init(&wc.cond));
|
||||
ASSERT(0 == uv_mutex_init(&wc.mutex));
|
||||
/* Helper to signal-then-wait. */
|
||||
worker_config_init(&wc, 1, condvar_signal, condvar_wait);
|
||||
ASSERT(0 == uv_thread_create(&thread, worker, &wc));
|
||||
|
||||
/* We wait-then-signal. */
|
||||
ASSERT(0 == wc.wait_cond(&wc, &wc.posted_1));
|
||||
wc.signal_cond(&wc, &wc.posted_2);
|
||||
|
||||
ASSERT(0 == uv_thread_join(&thread));
|
||||
uv_mutex_destroy(&wc.mutex);
|
||||
uv_cond_destroy(&wc.cond);
|
||||
worker_config_destroy(&wc);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
static int condvar_timedwait(worker_config* c, const volatile int* flag) {
|
||||
/* 1. Wait on a signal (hopefully not timeout, else we'll hang).
|
||||
* 2. Ensure that the signaler finished. */
|
||||
static int condvar_timedwait(worker_config* c, const int* flag) {
|
||||
int r;
|
||||
|
||||
r = 0;
|
||||
|
||||
uv_mutex_lock(&c->mutex);
|
||||
if (c->wait_delay)
|
||||
uv_sleep(c->wait_delay);
|
||||
while (*flag == 0) {
|
||||
r = uv_cond_timedwait(&c->cond, &c->mutex, (uint64_t)(150 * 1e6));
|
||||
ASSERT(r == 0 || r == UV_ETIMEDOUT);
|
||||
if (r == UV_ETIMEDOUT)
|
||||
break;
|
||||
}
|
||||
|
||||
/* Tell signal'er that I am waiting. */
|
||||
uv_sem_post(&c->sem_waiting);
|
||||
|
||||
/* Wait until I get a non-spurious signal. */
|
||||
do {
|
||||
r = uv_cond_timedwait(&c->cond, &c->mutex, (uint64_t)(1 * 1e9)); /* 1 s */
|
||||
ASSERT(r == 0); /* Should not time out. */
|
||||
} while (*flag == 0);
|
||||
ASSERT(*flag == 1);
|
||||
|
||||
uv_mutex_unlock(&c->mutex);
|
||||
|
||||
/* Wait for my signal'er to finish. */
|
||||
uv_sem_wait(&c->sem_signaled);
|
||||
return r;
|
||||
}
|
||||
|
||||
/* Test that uv_cond_timedwait will return early when cond is signaled. */
|
||||
/* uv_cond_timedwait: One thread signals, the other timedwaits. */
|
||||
TEST_IMPL(condvar_3) {
|
||||
uv_thread_t thread;
|
||||
worker_config wc;
|
||||
uv_thread_t thread;
|
||||
|
||||
memset(&wc, 0, sizeof(wc));
|
||||
wc.signal_delay = 100;
|
||||
wc.signal_cond = condvar_signal;
|
||||
wc.wait_cond = condvar_timedwait;
|
||||
|
||||
ASSERT(0 == uv_cond_init(&wc.cond));
|
||||
ASSERT(0 == uv_mutex_init(&wc.mutex));
|
||||
/* Helper to signal-then-wait. */
|
||||
worker_config_init(&wc, 0, condvar_signal, condvar_timedwait);
|
||||
ASSERT(0 == uv_thread_create(&thread, worker, &wc));
|
||||
|
||||
ASSERT(0 == wc.wait_cond(&wc, &wc.posted_1));
|
||||
/* We wait-then-signal. */
|
||||
wc.wait_cond(&wc, &wc.posted_1);
|
||||
wc.signal_cond(&wc, &wc.posted_2);
|
||||
|
||||
ASSERT(0 == uv_thread_join(&thread));
|
||||
uv_mutex_destroy(&wc.mutex);
|
||||
uv_cond_destroy(&wc.cond);
|
||||
worker_config_destroy(&wc);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
/* uv_cond_timedwait: One thread broadcasts, the other waits. */
|
||||
TEST_IMPL(condvar_4) {
|
||||
uv_thread_t thread;
|
||||
worker_config wc;
|
||||
uv_thread_t thread;
|
||||
|
||||
memset(&wc, 0, sizeof(wc));
|
||||
wc.signal_delay = 100;
|
||||
wc.signal_cond = condvar_signal;
|
||||
wc.wait_cond = condvar_timedwait;
|
||||
|
||||
ASSERT(0 == uv_cond_init(&wc.cond));
|
||||
ASSERT(0 == uv_mutex_init(&wc.mutex));
|
||||
/* Helper to signal-then-wait. */
|
||||
worker_config_init(&wc, 1, condvar_signal, condvar_timedwait);
|
||||
ASSERT(0 == uv_thread_create(&thread, worker, &wc));
|
||||
|
||||
/* We wait-then-signal. */
|
||||
wc.wait_cond(&wc, &wc.posted_1);
|
||||
wc.signal_cond(&wc, &wc.posted_2);
|
||||
|
||||
ASSERT(0 == uv_thread_join(&thread));
|
||||
uv_mutex_destroy(&wc.mutex);
|
||||
uv_cond_destroy(&wc.cond);
|
||||
worker_config_destroy(&wc);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
/* uv_cond_timedwait: One thread waits, no signal. Timeout should be delivered. */
|
||||
TEST_IMPL(condvar_5) {
|
||||
uv_thread_t thread;
|
||||
worker_config wc;
|
||||
|
||||
memset(&wc, 0, sizeof(wc));
|
||||
wc.use_broadcast = 1;
|
||||
wc.signal_delay = 100;
|
||||
wc.signal_cond = condvar_signal;
|
||||
wc.wait_cond = condvar_wait;
|
||||
|
||||
ASSERT(0 == uv_cond_init(&wc.cond));
|
||||
ASSERT(0 == uv_mutex_init(&wc.mutex));
|
||||
ASSERT(0 == uv_thread_create(&thread, worker, &wc));
|
||||
|
||||
wc.wait_cond(&wc, &wc.posted_1);
|
||||
wc.signal_cond(&wc, &wc.posted_2);
|
||||
|
||||
ASSERT(0 == uv_thread_join(&thread));
|
||||
uv_mutex_destroy(&wc.mutex);
|
||||
uv_cond_destroy(&wc.cond);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Test that uv_cond_timedwait will time out when cond is not signaled. */
|
||||
TEST_IMPL(condvar_6) {
|
||||
uv_thread_t thread;
|
||||
worker_config wc;
|
||||
int r;
|
||||
/* ns */
|
||||
uint64_t before;
|
||||
uint64_t after;
|
||||
uint64_t elapsed;
|
||||
uint64_t timeout;
|
||||
|
||||
memset(&wc, 0, sizeof(wc));
|
||||
wc.signal_delay = 100;
|
||||
wc.signal_cond = condvar_signal;
|
||||
wc.wait_cond = condvar_timedwait;
|
||||
timeout = 100 * 1e6; /* 100 ms in ns */
|
||||
|
||||
ASSERT(0 == uv_cond_init(&wc.cond));
|
||||
ASSERT(0 == uv_mutex_init(&wc.mutex));
|
||||
ASSERT(0 == uv_thread_create(&thread, noop_worker, &wc));
|
||||
/* Mostly irrelevant. We need cond and mutex initialized. */
|
||||
worker_config_init(&wc, 0, NULL, NULL);
|
||||
|
||||
/* This can only return having timed out, because otherwise we
|
||||
* loop forever in condvar_timedwait. */
|
||||
r = wc.wait_cond(&wc, &wc.posted_1);
|
||||
uv_mutex_lock(&wc.mutex);
|
||||
|
||||
/* We wait.
|
||||
* No signaler, so this will only return if timeout is delivered. */
|
||||
before = uv_hrtime();
|
||||
r = uv_cond_timedwait(&wc.cond, &wc.mutex, timeout);
|
||||
after = uv_hrtime();
|
||||
|
||||
uv_mutex_unlock(&wc.mutex);
|
||||
|
||||
/* It timed out. */
|
||||
ASSERT(r == UV_ETIMEDOUT);
|
||||
|
||||
ASSERT(0 == uv_thread_join(&thread));
|
||||
uv_mutex_destroy(&wc.mutex);
|
||||
uv_cond_destroy(&wc.cond);
|
||||
/* It must have taken at least timeout, modulo system timer ticks.
|
||||
* But it should not take too much longer.
|
||||
* cf. MSDN docs:
|
||||
* https://msdn.microsoft.com/en-us/library/ms687069(VS.85).aspx */
|
||||
elapsed = after - before;
|
||||
ASSERT(0.75 * timeout <= elapsed); /* 1.0 too large for Windows. */
|
||||
ASSERT(elapsed <= 1.5 * timeout); /* 1.1 too small for OSX. */
|
||||
|
||||
worker_config_destroy(&wc);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
@ -42,7 +42,6 @@ TEST_DECLARE (condvar_2)
|
||||
TEST_DECLARE (condvar_3)
|
||||
TEST_DECLARE (condvar_4)
|
||||
TEST_DECLARE (condvar_5)
|
||||
TEST_DECLARE (condvar_6)
|
||||
TEST_DECLARE (semaphore_1)
|
||||
TEST_DECLARE (semaphore_2)
|
||||
TEST_DECLARE (semaphore_3)
|
||||
@ -459,7 +458,6 @@ TASK_LIST_START
|
||||
TEST_ENTRY (condvar_3)
|
||||
TEST_ENTRY (condvar_4)
|
||||
TEST_ENTRY (condvar_5)
|
||||
TEST_ENTRY (condvar_6)
|
||||
TEST_ENTRY (semaphore_1)
|
||||
TEST_ENTRY (semaphore_2)
|
||||
TEST_ENTRY (semaphore_3)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user