[m-rev.] for review: fix bug #357: parallel conjunction broken on OS X

Julien Fischer jfischer at opturion.com
Sun Oct 2 00:13:54 AEST 2016


For review by anyone.

-------------------------------

Fix bug #357: parallel conjunction broken on OS X.

The parallel version of the runtime makes use of POSIX unnamed semaphores,
which do not exist on OS X (although annoyingly most of the relevant functions
*do* exist, they just don't do anything).  This was originally a problem for
the low-level C version of the parallel runtime, but now affects both C
backends (i.e. it's currently impossible to use threads or parallel
conjunctions on OS X).  The fix is to replace the use of POSIX unnamed
semaphores on OS X, with the semaphore implementation for libdispatch (which is
part of Grand Central Dispatch).  Note that the .par grades are (presumably)
still broken in versions of OS X prior to 10.6 -- they'll now just fail to
compile rather than fail at runtime.

configure.ac:
runtime/mercury_conf.h.in:
     Check whether libdispatch is present.

runtime/mercury_conf_param.h:
     Define a macro that says whether we want to use libdispatch; always
     define this macro on OS X.  Using libdispatch (optionally) on other systems
     is future work.

runtime/mercury_thread.{c,h}
     Define the MercurySem type and its associated operations appropriately
     when using libdispatch.

library/Mmakefile:
     Recompile the thread and thread.semaphore if any of the runtime headers
     that define the macros they use change.

Julien.

diff --git a/configure.ac b/configure.ac
index c6e6988..0ae491f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1346,7 +1346,7 @@ MERCURY_CHECK_FOR_HEADERS( \
          sys/types.h sys/stat.h fcntl.h termios.h sys/ioctl.h \
          sys/resource.h sys/stropts.h windows.h dirent.h getopt.h malloc.h \
          semaphore.h pthread.h time.h spawn.h fenv.h sys/mman.h sys/sem.h \
-        sched.h utmpx.h)
+        sched.h utmpx.h dispatch/dispatch.h)

  if test "$MR_HAVE_GETOPT_H" = 1; then
      GETOPT_H_AVAILABLE=yes
diff --git a/library/Mmakefile b/library/Mmakefile
index 8326596..8e84bad 100644
--- a/library/Mmakefile
+++ b/library/Mmakefile
@@ -418,6 +418,18 @@ $(os_subdir)par_builtin.pic_o \
  	../runtime/mercury_par_builtin.h \
  	../runtime/mercury_thread.h

+$(os_subdir)thread.$O \
+$(os_subdir)thread.pic_o \
+	: ../runtime/mercury_context.h \
+	../runtime/mercury_par_builtin.h \
+	../runtime/mercury_thread.h
+
+$(os_subdir)thread.semaphore.$O \
+$(os_subdir)thread.semaphore.pic_o \
+	: ../runtime/mercury_context.h \
+	../runtime/mercury_par_builtin.h \
+	../runtime/mercury_thread.h
+
  # robdd.m #includes both ../robdd/bryant.h and ../robdd/bryant.c
  # in foreign_decl and foreign_code respectively. They in turn
  # depend on ../robdd/robdd_conf.h.
diff --git a/runtime/mercury_conf.h.in b/runtime/mercury_conf.h.in
index f027c3d..3176987 100644
--- a/runtime/mercury_conf.h.in
+++ b/runtime/mercury_conf.h.in
@@ -138,6 +138,7 @@
  //  MR_HAVE_SCHED_H         we have <sched.h>
  //  MR_HAVE_UTMPX_H         we have <utmpx.h>
  //  MR_HAVE_SYS_RESOURCE_H  we have <sys/resource.h>
+//  MR_HAVE_DISPATCH_DISPATCH_H we have <dispatch/dispatch.h>

  #undef  MR_HAVE_SYS_SIGINFO_H
  #undef  MR_HAVE_SYS_SIGNAL_H
@@ -170,6 +171,7 @@
  #undef  MR_HAVE_SCHED_H
  #undef  MR_HAVE_UTMPX_H
  #undef  MR_HAVE_SYS_RESOURCE_H
+#undef  MR_HAVE_DISPATCH_DISPATCH_H

  // MR_HAVE_POSIX_TIMES is defined if we have the POSIX
  // `struct tms' struct and times() function.
diff --git a/runtime/mercury_conf_param.h b/runtime/mercury_conf_param.h
index ec03693..75fe0cb 100644
--- a/runtime/mercury_conf_param.h
+++ b/runtime/mercury_conf_param.h
@@ -51,8 +51,15 @@

  // Mac OS X specific.

+// MR_USE_LIBDISPATCH is defined if we should use libdispatch to provide
+// concurrency operations.  Currently, we only do this on OS X.  In the
+// future, we may do so for other OSs (e.g. FreeBSD).
+
  #if defined(__APPLE__) && defined(__MACH__)
    #define MR_MAC_OSX
+  #if defined(MR_HAVE_DISPATCH_DISPATCH_H)
+     #define MR_USE_LIBDISPATCH
+  #endif
  #endif

  ////////////////////////////////////////////////////////////////////////////
diff --git a/runtime/mercury_thread.c b/runtime/mercury_thread.c
index 16129ea..023e761 100644
--- a/runtime/mercury_thread.c
+++ b/runtime/mercury_thread.c
@@ -415,10 +415,20 @@ MR_cond_timed_wait(MercuryCond *cond, MercuryLock *lock,
  void
  MR_sem_init(MercurySem *sem, unsigned int value)
  {
+#if defined(MR_USE_LIBDISPATCH)
+    *sem = dispatch_semaphore_create(value);
+    if (*sem == NULL) {
+        MR_perror("cannot initialize semaphore");
+        exit(EXIT_FAILURE);
+    }
+#else // !MR_USE_LIBDISPATCH
+    // XXX we should check errno and say *why* we could not initialize
+    // the semaphore.
      if (sem_init(sem, 0, value) == -1) {
          MR_perror("cannot initialize semaphore");
          exit(EXIT_FAILURE);
      }
+#endif
  }

  int
@@ -430,7 +440,11 @@ MR_sem_wait(MercurySem *sem, const char *from)
          "%" MR_INTEGER_LENGTH_MODIFIER "d waiting on sem: %p (%s)\n",
          MR_SELF_THREAD_ID, sem, from);
      fflush(stderr);
+#if defined(MR_USE_LIBDISPATCH)
+    err = dispatch_semaphore_wait(*sem, DISPATCH_TIME_FOREVER);
+#else
      err = sem_wait(sem);
+#endif
      fprintf(stderr,
          "%" MR_INTEGER_LENGTH_MODIFIER "d wait returned %d\n",
          MR_SELF_THREAD_ID, err);
@@ -449,8 +463,8 @@ MR_sem_timed_wait(MercurySem *sem, const struct timespec *abstime,
          "%" MR_INTEGER_LENGTH_MODIFIER "d timed wait on sem: %p (%s)\n",
              MR_SELF_THREAD_ID, sem, from);
      fflush(stderr);
-#if defined(MR_MAC_OSX)
-    MR_fatal_error("sem_timedwait not supported on Mac OS X");
+#if defined(MR_USE_LIBDISPATCH)
+    err = dispatch_semaphore_wait(*sem, dispatch_walltime(abstime, 0));
  #else
      err = sem_timedwait(sem, abstime);
  #endif
@@ -471,7 +485,11 @@ MR_sem_post(MercurySem *sem, const char *from)
          "%" MR_INTEGER_LENGTH_MODIFIER "d posting to sem: %p (%s)\n",
          MR_SELF_THREAD_ID, sem, from);
      fflush(stderr);
+#if defined(MR_USE_LIBDISPATCH)
+    err = dispatch_semaphore_signal(*sem);
+#else
      err = sem_post(sem);
+#endif
      fprintf(stderr,
          "%" MR_INTEGER_LENGTH_MODIFIER "d post returned %d\n",
          MR_SELF_THREAD_ID, err);
@@ -483,10 +501,14 @@ MR_sem_post(MercurySem *sem, const char *from)
  void
  MR_sem_destroy(MercurySem *sem)
  {
+#if defined(MR_USE_LIBDISPATCH)
+   dispatch_release(*sem);
+#else
     if (sem_destroy(sem) == -1) {
          MR_perror("cannot destroy semaphore");
          exit(EXIT_FAILURE);
      }
+#endif
  }

  // For pthreads-win32 MR_null_thread() is defined as follows. For other
diff --git a/runtime/mercury_thread.h b/runtime/mercury_thread.h
index 2dcaaa1..c518b35 100644
--- a/runtime/mercury_thread.h
+++ b/runtime/mercury_thread.h
@@ -1,7 +1,7 @@
  // vim: ts=4 sw=4 expandtab ft=c

  // Copyright (C) 1997-1998, 2000, 2003, 2005-2007, 2009-2011 The University of Melbourne.
-// Copyright (C) 2014 The Mercury team.
+// Copyright (C) 2014-2016 The Mercury team.
  // This file may only be copied under the terms of the GNU Library General
  // Public License - see the file COPYING.LIB in the Mercury distribution.

@@ -14,7 +14,12 @@

    #include <signal.h>   // for sigset_t on the SPARC
    #include <pthread.h>
-  #include <semaphore.h> // POSIX semaphores
+
+  #if defined(MR_USE_LIBDISPATCH)
+      #include <dispatch/dispatch.h>
+  #else
+      #include <semaphore.h> // POSIX semaphores
+  #endif

    #define MR_MUTEX_ATTR       NULL
    #define MR_COND_ATTR        NULL
@@ -25,7 +30,11 @@
    typedef pthread_mutex_t   MercuryLock;
    typedef pthread_cond_t    MercuryCond;

-  typedef sem_t             MercurySem;
+  #if defined(MR_USE_LIBDISPATCH)
+    typedef dispatch_semaphore_t MercurySem;
+  #else
+    typedef sem_t             MercurySem;
+  #endif

    extern int    MR_mutex_lock(MercuryLock *lock, const char *from);
    extern int    MR_mutex_unlock(MercuryLock *lock, const char *from);
@@ -60,9 +69,10 @@
    extern MR_bool    MR_debug_threads;

    #ifndef MR_DEBUG_THREADS
-    // The following macros should be used once the use of locking
-    // in the generated code is considered stable, since the alternative
-    // versions do the same thing, but with debugging support.
+
+    // The following macros should be used once the use of locking in the
+    // generated code is considered stable, since the alternative versions do
+    // the same thing, but with debugging support enabled.

      #define MR_LOCK(lck, from)      pthread_mutex_lock((lck))
      #define MR_UNLOCK(lck, from)    pthread_mutex_unlock((lck))
@@ -73,11 +83,21 @@
      #define MR_TIMED_WAIT(cond, mtx, abstime, from)                     \
          pthread_cond_timedwait((cond), (mtx), (abstime))

-    #define MR_SEM_WAIT(sem, from)  sem_wait((sem))
-    #define MR_SEM_POST(sem, from)  sem_post((sem))
-    #define MR_SEM_TIMED_WAIT(sem, abstime, from)                       \
+    #if defined(MR_USE_LIBDISPATCH)
+      #define MR_SEM_WAIT(sem, from)                                    \
+        dispatch_semaphore_wait(*(sem), DISPATCH_TIME_FOREVER)
+      #define MR_SEM_POST(sem, from)  dispatch_semaphore_signal(*(sem))
+      #define MR_SEM_TIMED_WAIT(sem, abstime, from)                     \
+        dispatch_semaphore_wait(*(sem), dispatch_walltime((abstime))
+    #else
+      #define MR_SEM_WAIT(sem, from)  sem_wait((sem))
+      #define MR_SEM_POST(sem, from)  sem_post((sem))
+      #define MR_SEM_TIMED_WAIT(sem, abstime, from)                     \
          sem_timedwait((sem), (abstime))
-  #else
+    #endif // !MR_USE_LIBDISPATCH
+
+  #else // MR_DEBUG_THREADS
+
      #define MR_LOCK(lck, from)                                          \
                  ( MR_debug_threads ?                                    \
                      MR_mutex_lock((lck), (from))                        \
@@ -116,37 +136,55 @@
              pthread_cond_timedwait((cond), (mtx), (abstime))            \
          )

-    #define MR_SEM_WAIT(sem, from)                                      \
-        ( MR_debug_threads ?                                            \
-            MR_sem_wait((sem), (from))                                  \
-        :                                                               \
-            sem_wait((sem))                                             \
-        )
-
-#if defined(MR_MAC_OSX)
-    #define MR_SEM_TIMED_WAIT(sem, abstime, from)                       \
-        ( MR_debug_threads ?                                            \
-            MR_sem_timed_wait((sem), (abstime), (from))                 \
-        :                                                               \
-            MR_fatal_error("sem_timedwait not supported on Mac OS X")   \
-        )
-#else
-    #define MR_SEM_TIMED_WAIT(sem, abstime, from)                       \
-        ( MR_debug_threads ?                                            \
-            MR_sem_timed_wait((sem), (abstime), (from))                 \
-        :                                                               \
-            sem_timedwait((sem), (abstime))                             \
-        )
-#endif
-
-    #define MR_SEM_POST(sem, from)                                      \
-        ( MR_debug_threads ?                                            \
-            MR_sem_post((sem), (from))                                  \
-        :                                                               \
-            sem_post((sem))                                             \
-        )
-
-  #endif
+    #if defined(MR_USE_LIBDISPATCH)
+
+      #define MR_SEM_WAIT(sem, from)                                      \
+          ( MR_debug_threads ?                                            \
+              MR_sem_wait((sem), (from))                                  \
+          :                                                               \
+              dispatch_semaphore_wait(*(sem), DISPATCH_TIME_FOREVER)      \
+          )
+
+      #define MR_SEM_TIMED_WAIT(sem, abstime, from)                       \
+          ( MR_debug_threads ?                                            \
+              MR_sem_timed_wait((sem), (abstime), (from))                 \
+          :                                                               \
+              dispatch_semaphore_wait(*(sem), dispatch_walltime((abstime), 0)) \
+          )
+
+      #define MR_SEM_POST(sem, from)                                      \
+          ( MR_debug_threads ?                                            \
+              MR_sem_post((sem), (from))                                  \
+          :                                                               \
+              dispatch_semaphore_signal(*(sem))                           \
+          )
+
+    #else // !MR_USE_LIBDISPATCH
+
+      #define MR_SEM_WAIT(sem, from)                                      \
+          ( MR_debug_threads ?                                            \
+              MR_sem_wait((sem), (from))                                  \
+          :                                                               \
+              sem_wait((sem))                                             \
+          )
+
+      #define MR_SEM_TIMED_WAIT(sem, abstime, from)                       \
+          ( MR_debug_threads ?                                            \
+              MR_sem_timed_wait((sem), (abstime), (from))                 \
+          :                                                               \
+              sem_timedwait((sem), (abstime))                             \
+          )
+
+      #define MR_SEM_POST(sem, from)                                      \
+          ( MR_debug_threads ?                                            \
+              MR_sem_post((sem), (from))                                  \
+          :                                                               \
+              sem_post((sem))                                             \
+          )
+
+    #endif // !MR_USE_LIBDISPATCH
+
+  #endif // MR_DEBUG_THREADS

    // The following two macros are used to protect pragma foreign_proc
    // predicates which are not thread-safe.


More information about the reviews mailing list