Skip to content

Use a custom condition variable implementation #787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 12, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 94 additions & 18 deletions ocaml/otherlibs/systhreads/st_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@
#include <time.h>
#include <sys/time.h>
#ifdef __linux__
#include <features.h>
#include <unistd.h>
#include <sys/syscall.h>
#include <linux/futex.h>
#include <limits.h>
#endif

typedef int st_retcode;
Expand Down Expand Up @@ -109,6 +113,78 @@ Caml_inline void st_thread_set_id(intnat id)
return;
}

/* If we're using glibc, use a custom condition variable implementation to
avoid this bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25847

For now we only have this on linux because it directly uses the linux futex
syscalls. */
#if defined(__linux__) && defined(__GNU_LIBRARY__) && defined(__GLIBC__) && defined(__GLIBC_MINOR__)
typedef struct {
volatile unsigned counter;
} custom_condvar;

static int custom_condvar_init(custom_condvar * cv)
{
cv->counter = 0;
return 0;
}

static int custom_condvar_destroy(custom_condvar * cv)
{
return 0;
}

static int custom_condvar_wait(custom_condvar * cv, pthread_mutex_t * mutex)
{
unsigned old_count = cv->counter;
pthread_mutex_unlock(mutex);
syscall(SYS_futex, &cv->counter, FUTEX_WAIT_PRIVATE, old_count, NULL, NULL, 0);
pthread_mutex_lock(mutex);
return 0;
}

static int custom_condvar_signal(custom_condvar * cv)
{
__sync_add_and_fetch(&cv->counter, 1);
syscall(SYS_futex, &cv->counter, FUTEX_WAKE_PRIVATE, 1, NULL, NULL, 0);
return 0;
}

static int custom_condvar_broadcast(custom_condvar * cv)
{
__sync_add_and_fetch(&cv->counter, 1);
syscall(SYS_futex, &cv->counter, FUTEX_WAKE_PRIVATE, INT_MAX, NULL, NULL, 0);
return 0;
}
#else
typedef pthread_cond_t custom_condvar;

static int custom_condvar_init(custom_condvar * cv)
{
return pthread_cond_init(cv, NULL);
}

static int custom_condvar_destroy(custom_condvar * cv)
{
return pthread_cond_destroy(cv);
}

static int custom_condvar_wait(custom_condvar * cv, pthread_mutex_t * mutex)
{
return pthread_cond_wait(cv, mutex);
}

static int custom_condvar_signal(custom_condvar * cv)
{
return pthread_cond_signal(cv);
}

static int custom_condvar_broadcast(custom_condvar * cv)
{
return pthread_cond_broadcast(cv);
}
#endif

/* The master lock. This is a mutex that is held most of the time,
so we implement it in a slightly convoluted way to avoid
all risks of busy-waiting. Also, we count the number of waiting
Expand All @@ -118,13 +194,13 @@ typedef struct {
pthread_mutex_t lock; /* to protect contents */
int busy; /* 0 = free, 1 = taken */
volatile int waiters; /* number of threads waiting on master lock */
pthread_cond_t is_free; /* signaled when free */
custom_condvar is_free; /* signaled when free */
} st_masterlock;

static void st_masterlock_init(st_masterlock * m)
{
pthread_mutex_init(&m->lock, NULL);
pthread_cond_init(&m->is_free, NULL);
custom_condvar_init(&m->is_free);
m->busy = 1;
m->waiters = 0;
}
Expand All @@ -134,7 +210,7 @@ static void st_masterlock_acquire(st_masterlock * m)
pthread_mutex_lock(&m->lock);
while (m->busy) {
m->waiters ++;
pthread_cond_wait(&m->is_free, &m->lock);
custom_condvar_wait(&m->is_free, &m->lock);
m->waiters --;
}
m->busy = 1;
Expand All @@ -146,7 +222,7 @@ static void st_masterlock_release(st_masterlock * m)
pthread_mutex_lock(&m->lock);
m->busy = 0;
pthread_mutex_unlock(&m->lock);
pthread_cond_signal(&m->is_free);
custom_condvar_signal(&m->is_free);
}

CAMLno_tsan /* This can be called for reading [waiters] without locking. */
Expand Down Expand Up @@ -179,14 +255,14 @@ Caml_inline void st_thread_yield(st_masterlock * m)
}

m->busy = 0;
pthread_cond_signal(&m->is_free);
custom_condvar_signal(&m->is_free);
m->waiters++;
do {
/* Note: the POSIX spec prevents the above signal from pairing with this
wait, which is good: we'll reliably continue waiting until the next
yield() or enter_blocking_section() call (or we see a spurious condvar
wakeup, which are rare at best.) */
pthread_cond_wait(&m->is_free, &m->lock);
custom_condvar_wait(&m->is_free, &m->lock);
} while (m->busy);
m->busy = 1;
m->waiters--;
Expand Down Expand Up @@ -254,14 +330,14 @@ Caml_inline int st_mutex_unlock(st_mutex m)

/* Condition variables */

typedef pthread_cond_t * st_condvar;
typedef custom_condvar * st_condvar;

static int st_condvar_create(st_condvar * res)
{
int rc;
st_condvar c = caml_stat_alloc_noexc(sizeof(pthread_cond_t));
st_condvar c = caml_stat_alloc_noexc(sizeof(custom_condvar));
if (c == NULL) return ENOMEM;
rc = pthread_cond_init(c, NULL);
rc = custom_condvar_init(c);
if (rc != 0) { caml_stat_free(c); return rc; }
*res = c;
return 0;
Expand All @@ -270,32 +346,32 @@ static int st_condvar_create(st_condvar * res)
static int st_condvar_destroy(st_condvar c)
{
int rc;
rc = pthread_cond_destroy(c);
rc = custom_condvar_destroy(c);
caml_stat_free(c);
return rc;
}

Caml_inline int st_condvar_signal(st_condvar c)
{
return pthread_cond_signal(c);
return custom_condvar_signal(c);
}

Caml_inline int st_condvar_broadcast(st_condvar c)
{
return pthread_cond_broadcast(c);
return custom_condvar_broadcast(c);
}

Caml_inline int st_condvar_wait(st_condvar c, st_mutex m)
{
return pthread_cond_wait(c, m);
return custom_condvar_wait(c, m);
}

/* Triggered events */

typedef struct st_event_struct {
pthread_mutex_t lock; /* to protect contents */
int status; /* 0 = not triggered, 1 = triggered */
pthread_cond_t triggered; /* signaled when triggered */
custom_condvar triggered; /* signaled when triggered */
} * st_event;

static int st_event_create(st_event * res)
Expand All @@ -305,7 +381,7 @@ static int st_event_create(st_event * res)
if (e == NULL) return ENOMEM;
rc = pthread_mutex_init(&e->lock, NULL);
if (rc != 0) { caml_stat_free(e); return rc; }
rc = pthread_cond_init(&e->triggered, NULL);
rc = custom_condvar_init(&e->triggered);
if (rc != 0)
{ pthread_mutex_destroy(&e->lock); caml_stat_free(e); return rc; }
e->status = 0;
Expand All @@ -317,7 +393,7 @@ static int st_event_destroy(st_event e)
{
int rc1, rc2;
rc1 = pthread_mutex_destroy(&e->lock);
rc2 = pthread_cond_destroy(&e->triggered);
rc2 = custom_condvar_destroy(&e->triggered);
caml_stat_free(e);
return rc1 != 0 ? rc1 : rc2;
}
Expand All @@ -330,7 +406,7 @@ static int st_event_trigger(st_event e)
e->status = 1;
rc = pthread_mutex_unlock(&e->lock);
if (rc != 0) return rc;
rc = pthread_cond_broadcast(&e->triggered);
rc = custom_condvar_broadcast(&e->triggered);
return rc;
}

Expand All @@ -340,7 +416,7 @@ static int st_event_wait(st_event e)
rc = pthread_mutex_lock(&e->lock);
if (rc != 0) return rc;
while(e->status == 0) {
rc = pthread_cond_wait(&e->triggered, &e->lock);
rc = custom_condvar_wait(&e->triggered, &e->lock);
if (rc != 0) return rc;
}
rc = pthread_mutex_unlock(&e->lock);
Expand Down