flambda-backend: Port "condvar" bugfix to runtime 5 (#2076)

Author: xclerc

otherlibs/systhreads/st_pthreads.h

@@ -100,15 +100,15 @@ typedef struct {
   pthread_mutex_t lock;           /* to protect contents */
   uintnat busy;                   /* 0 = free, 1 = taken */
   atomic_uintnat 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)
 {
   if (!m->init) {
     // FIXME: check errors
     pthread_mutex_init(&m->lock, NULL);
-    pthread_cond_init(&m->is_free, NULL);
+    custom_condvar_init(&m->is_free);
     m->init = 1;
   }
   m->busy = 1;
@@ -156,7 +156,7 @@ static void st_masterlock_acquire(st_masterlock *m)
   pthread_mutex_lock(&m->lock);
   while (m->busy) {
     atomic_fetch_add(&m->waiters, +1);
-    pthread_cond_wait(&m->is_free, &m->lock);
+    custom_condvar_wait(&m->is_free, &m->lock);
     atomic_fetch_add(&m->waiters, -1);
   }
   m->busy = 1;
@@ -171,7 +171,7 @@ static void st_masterlock_release(st_masterlock * m)
   pthread_mutex_lock(&m->lock);
   m->busy = 0;
   st_bt_lock_release(m);
-  pthread_cond_signal(&m->is_free);
+  custom_condvar_signal(&m->is_free);
   pthread_mutex_unlock(&m->lock);
 
   return;
@@ -203,7 +203,7 @@ Caml_inline void st_thread_yield(st_masterlock * m)
 
   m->busy = 0;
   atomic_fetch_add(&m->waiters, +1);
-  pthread_cond_signal(&m->is_free);
+  custom_condvar_signal(&m->is_free);
   /* releasing the domain lock but not triggering bt messaging
      messaging the bt should not be required because yield assumes
      that a thread will resume execution (be it the yielding thread
@@ -215,7 +215,7 @@ Caml_inline void st_thread_yield(st_masterlock * m)
        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;
@@ -233,7 +233,7 @@ Caml_inline void st_thread_yield(st_masterlock * m)
 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;
 
 
@@ -244,7 +244,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;
@@ -256,7 +256,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;
 }
@@ -269,7 +269,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;
 }
 
@@ -279,7 +279,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);

runtime/caml/platform.h

@@ -25,6 +25,7 @@
 #include <string.h>
 #include "config.h"
 #include "mlvalues.h"
+#include "sync.h"
 #include "sys.h"
 
 #if defined(MAP_ANON) && !defined(MAP_ANONYMOUS)
@@ -106,7 +107,7 @@ void caml_plat_assert_locked(caml_plat_mutex*);
 void caml_plat_assert_all_locks_unlocked(void);
 Caml_inline void caml_plat_unlock(caml_plat_mutex*);
 void caml_plat_mutex_free(caml_plat_mutex*);
-typedef struct { pthread_cond_t cond; caml_plat_mutex* mutex; } caml_plat_cond;
+typedef struct { custom_condvar cond; caml_plat_mutex* mutex; } caml_plat_cond;
 #define CAML_PLAT_COND_INITIALIZER(m) { PTHREAD_COND_INITIALIZER, m }
 void caml_plat_cond_init(caml_plat_cond*, caml_plat_mutex*);
 void caml_plat_wait(caml_plat_cond*);

runtime/caml/sync.h

@@ -29,6 +29,19 @@ typedef pthread_mutex_t * sync_mutex;
 CAMLextern int caml_mutex_lock(sync_mutex mut);
 CAMLextern int caml_mutex_unlock(sync_mutex mut);
 
+/* 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;
+#else
+typedef pthread_cond_t custom_condvar;
+#endif
+
 #endif /* CAML_INTERNALS */
 
 #endif /* CAML_SYNC_H */

runtime/platform.c

@@ -33,6 +33,9 @@
 #include "caml/domain.h"
 #endif
 
+#include "caml/alloc.h"
+#include "sync_posix.h"
+
 /* Error reporting */
 
 void caml_plat_fatal_error(const char * action, int err)
@@ -89,14 +92,7 @@ void caml_plat_mutex_free(caml_plat_mutex* m)
 
 static void caml_plat_cond_init_aux(caml_plat_cond *cond)
 {
-  pthread_condattr_t attr;
-  pthread_condattr_init(&attr);
-#if defined(_POSIX_TIMERS) && \
-    defined(_POSIX_MONOTONIC_CLOCK) && \
-    _POSIX_MONOTONIC_CLOCK != (-1)
-  pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
-#endif
-  pthread_cond_init(&cond->cond, &attr);
+  custom_condvar_init(&cond->cond);
 }
 
 /* Condition variables */
@@ -109,24 +105,24 @@ void caml_plat_cond_init(caml_plat_cond* cond, caml_plat_mutex* m)
 void caml_plat_wait(caml_plat_cond* cond)
 {
   caml_plat_assert_locked(cond->mutex);
-  check_err("wait", pthread_cond_wait(&cond->cond, cond->mutex));
+  check_err("wait", custom_condvar_wait(&cond->cond, cond->mutex));
 }
 
 void caml_plat_broadcast(caml_plat_cond* cond)
 {
   caml_plat_assert_locked(cond->mutex);
-  check_err("cond_broadcast", pthread_cond_broadcast(&cond->cond));
+  check_err("cond_broadcast", custom_condvar_broadcast(&cond->cond));
 }
 
 void caml_plat_signal(caml_plat_cond* cond)
 {
   caml_plat_assert_locked(cond->mutex);
-  check_err("cond_signal", pthread_cond_signal(&cond->cond));
+  check_err("cond_signal", custom_condvar_signal(&cond->cond));
 }
 
 void caml_plat_cond_free(caml_plat_cond* cond)
 {
-  check_err("cond_free", pthread_cond_destroy(&cond->cond));
+  check_err("cond_free", custom_condvar_destroy(&cond->cond));
   cond->mutex=0;
 }
 

runtime/sync_posix.h

@@ -23,6 +23,14 @@
 #include <pthread.h>
 #include <string.h>
 
+#ifdef __linux__
+#include <features.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <linux/futex.h>
+#include <limits.h>
+#endif
+
 typedef int sync_retcode;
 
 /* Mutexes */
@@ -82,18 +90,92 @@ Caml_inline int sync_mutex_unlock(sync_mutex m)
   return pthread_mutex_unlock(m);
 }
 
+/* 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__)
+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
+static int custom_condvar_init(custom_condvar * cv)
+{
+  pthread_condattr_t attr;
+  pthread_condattr_init(&attr);
+#if defined(_POSIX_TIMERS) &&                   \
+  defined(_POSIX_MONOTONIC_CLOCK) &&            \
+  _POSIX_MONOTONIC_CLOCK != (-1)
+  pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
+#endif
+  return pthread_cond_init(cv, &attr);
+}
+
+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
+
+
 /* Condition variables */
 
-typedef pthread_cond_t * sync_condvar;
+typedef custom_condvar * sync_condvar;
 
 #define Condition_val(v) (* (sync_condvar *) Data_custom_val(v))
 
 Caml_inline int sync_condvar_create(sync_condvar * res)
 {
   int rc;
-  sync_condvar c = caml_stat_alloc_noexc(sizeof(pthread_cond_t));
+  sync_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;
@@ -102,24 +184,24 @@ Caml_inline int sync_condvar_create(sync_condvar * res)
 Caml_inline int sync_condvar_destroy(sync_condvar c)
 {
   int rc;
-  rc = pthread_cond_destroy(c);
+  rc = custom_condvar_destroy(c);
   caml_stat_free(c);
   return rc;
 }
 
 Caml_inline int sync_condvar_signal(sync_condvar c)
 {
- return pthread_cond_signal(c);
+  return custom_condvar_signal(c);
 }
 
 Caml_inline int sync_condvar_broadcast(sync_condvar c)
 {
-    return pthread_cond_broadcast(c);
+  return custom_condvar_broadcast(c);
 }
 
 Caml_inline int sync_condvar_wait(sync_condvar c, sync_mutex m)
 {
-  return pthread_cond_wait(c, m);
+  return custom_condvar_wait(c, m);
 }
 
 /* Reporting errors */