Subject: Re: no pthread_spinlock_t on Mac OS 10.6.4

From: "Gary V. Vaughan" <[email-masked]>
Date: Wed, 22 Sep 2010 12:11:11 +0700

Hi Paul,

On 21 Sep 2010, at 14:49, Paul Eggert wrote:
> Could you try the following patch instead?  It tries an idea discussed
> earlier today, namely, fall back on mutexes if spinlocks aren't there.
> You'll need not only to update lib/pthread.in.h and m4/pthread.m4, but
> also to get that code from modules/pthread into your makefile; I did
> that by manually editing the "begin gnulib module pthread" section of
> lib/gnulib.mk.

I like this idea much better than disabling threads entirely on macosx.

But the patch causes a bizarre and apparently unrelated compilation
error:

  ; make V=1
  gcc -std=gnu99  -I. -I../lib      -g -O2 -MT sigprocmask.o -MD -MP -MF .deps/sigprocmask.Tpo -c -o sigprocmask.o ../lib/sigprocmask.c
  ../lib/sigprocmask.c:87: error: expected identifier or '(' before 'const'
  ../lib/sigprocmask.c:87: error: expected ')' before '&' token
  ../lib/sigprocmask.c:87: error: expected ')' before '!=' token
  ../lib/sigprocmask.c:103: error: expected ')' before '*' token
  ../lib/sigprocmask.c:103: error: expected ')' before '=' token
  ../lib/sigprocmask.c:110: error: expected ')' before '*' token
  ../lib/sigprocmask.c:110: error: expected ')' before '|=' token
  ../lib/sigprocmask.c:130: error: expected ')' before '*' token
  ../lib/sigprocmask.c:130: error: expected ')' before '&=' token
  ../lib/sigprocmask.c:151: error: expected ')' before '*' token
  ../lib/sigprocmask.c:151: error: expected ')' before 

Apparently caused by an errant sigprocmask functions sharing names with
system macros, to create this mess:

  ; gcc -std=c99 -E ../lib/sigprocmask.c | sed '/^$/d' | cat -n
  [[...]]
  2376  int
  2377  ((*(const sigset_t *set) & __sigbits(int sig)) != 0)
  2378  {
  2379    if (sig >= 0 && sig < 32)
  2380      {
  2381        return (*set >> sig) & 1;
  2382      }
  2383    else
  2384      return 0;
  2385  }
  2386  int
  2387  (*(sigset_t *set) = 0, 0)
  2388  {
  2389    *set = 0;
  2390    return 0;
  2391  }
  2392  int
  2393  (*(sigset_t *set) |= __sigbits(int sig), 0)
  2394  {
  2395    if (sig >= 0 && sig < 32)
  2396      {
  2397        *set |= 1U << sig;
  2398        return 0;
  2399      }
  2400    else
  2401      {
  2402        (*__error()) = 22;
  2403        return -1;
  2404      }
  2405  }
  2406  int
  2407  (*(sigset_t *set) &= ~__sigbits(int sig), 0)
  2408  {
  2409    if (sig >= 0 && sig < 32)
  2410      {
  2411        *set &= ~(1U << sig);
  2412        return 0;
  2413      }
  2414    else
  2415      {
  2416        (*__error()) = 22;
  2417        return -1;
  2418      }
  2419  }
  2420  int
  2421  (*(sigset_t *set) = ~(sigset_t)0, 0)
  2422  {
  2423    *set = ((2U << (32 - 1)) - 1) & ~ 0;
  2424    return 0;
  2425  }

I suppose this is a result of header ordering?  The only things that have
changed since my previous report are that I've applied the patch below,
tweaked the generated gnulib.mk as suggested, and updated the gnulib
submodule to the tip of master so that the patch can apply.

I tried removing the `# include_next <pthread.h>' but the above error
still stands even then.  So maybe this is an entirely new problem with
latest gnulib?

> Sorry, I can't easily test this as I don't use MacOS.

No problem, glad to help. Sorry I couldn't unravel this any further by
myself.

> diff --git a/lib/pthread.in.h b/lib/pthread.in.h
> index 0fdf9c3..f8e358b 100644
> --- a/lib/pthread.in.h
> +++ b/lib/pthread.in.h
> @@ -19,6 +19,17 @@
> /* Written by Paul Eggert and Glen Lenker.  */
> 
> #ifndef _GL_PTHREAD_H_
> +
> +#if __GNUC__ >= 3
> +@PRAGMA_SYSTEM_HEADER@
> +#endif
> +
> +/* The include_next requires a split double-inclusion guard.  */
> +#if @HAVE_PTHREAD_H@
> +# @INCLUDE_NEXT@ @NEXT_PTHREAD_H@
> +#endif
> +
> +#ifndef _GL_PTHREAD_H_
> #define _GL_PTHREAD_H_
> 
> #include <errno.h>
> @@ -27,7 +38,7 @@
> #include <sys/types.h>
> #include <time.h>
> 
> -#ifndef HAVE_PTHREAD_T
> +#if ! @HAVE_PTHREAD_T@
>  typedef int pthread_t;
>  typedef int pthread_attr_t;
>  typedef int pthread_barrier_t;
> @@ -40,9 +51,9 @@
>  typedef int pthread_once_t;
>  typedef int pthread_rwlock_t;
>  typedef int pthread_rwlockattr_t;
> - typedef int pthread_spinlock_t;
> #endif
> 
> +#ifndef PTHREAD_COND_INITIALIZER
> #define PTHREAD_COND_INITIALIZER { 0 }
> #define PTHREAD_MUTEX_INITIALIZER { 0 }
> #define PTHREAD_ONCE_INIT { 0 }
> @@ -81,6 +92,9 @@
> 
> #define PTHREAD_SCOPE_SYSTEM 0
> #define PTHREAD_SCOPE_PROCESS 1
> +#endif
> +
> +#if ! @HAVE_PTHREAD_T@
> 
> /* Provide substitutes for the thread functions that should work
>    adequately on a single-threaded implementation, where
> @@ -147,6 +161,24 @@ pthread_join (pthread_t thread, void **pvalue)
> }
> 
> static inline int
> +pthread_mutexattr_destroy (pthread_mutexattr_t *attr)
> +{
> +  return 0;
> +}
> +
> +static inline int
> +pthread_mutexattr_init (pthread_mutexattr_t *attr)
> +{
> +  return 0;
> +}
> +
> +static inline int
> +pthread_mutexattr_settype (pthread_mutexattr_t *attr, int attr_type)
> +{
> +  return 0;
> +}
> +
> +static inline int
> pthread_mutex_destroy (pthread_mutex_t *mutex)
> {
>   /* MUTEX is never seriously used.  */
> @@ -170,6 +202,12 @@ pthread_mutex_lock (pthread_mutex_t *mutex)
> }
> 
> static inline int
> +pthread_mutex_trylock (pthread_mutex_t *mutex)
> +{
> +  return pthread_mutex_lock (mutex);
> +}
> +
> +static inline int
> pthread_mutex_unlock (pthread_mutex_t *mutex)
> {
>   /* There is only one thread, so it always unlocks successfully.
> @@ -178,40 +216,44 @@ pthread_mutex_unlock (pthread_mutex_t *mutex)
>   return 0;
> }
> 
> +#endif
> +
> +#if ! @HAVE_PTHREAD_SPINLOCK_T@
> +
> +/* Approximate spinlocks with mutexes.  */
> +
> +typedef pthread_mutex_t pthread_spinlock_t;
> +
> static inline int
> pthread_spin_init (pthread_spinlock_t *lock, int pshared)
> {
> -  /* LOCK is never seriously used.  */
> -  return 0;
> +  return pthread_mutex_init (lock, NULL);
> }
> 
> static inline int
> pthread_spin_destroy (pthread_spinlock_t *lock)
> {
> -  /* LOCK is never seriously used.  */
> -  return 0;
> +  return pthread_mutex_destroy (lock);
> }
> 
> static inline int
> pthread_spin_lock (pthread_spinlock_t *lock)
> {
> -  /* Only one thread, so it always gets the lock.  */
> -  return 0;
> +  return pthread_mutex_lock (lock);
> }
> 
> static inline int
> pthread_spin_trylock (pthread_spinlock_t *lock)
> {
> -  /* Only one thread, so it always gets the lock.  Assume that a
> -     thread never tries a lock that it already holds.  */
> -  return 0;
> +  return pthread_mutex_trylock (lock);
> }
> 
> static inline int
> pthread_spin_unlock (pthread_spinlock_t *lock)
> {
> -  /* Only one thread, so spin locks are no-ops.  */
> -  return 0;
> +  return pthread_mutex_unlock (lock);
> }
> +#endif
> 
> #endif /* _GL_PTHREAD_H_ */
> +#endif /* _GL_PTHREAD_H_ */
> diff --git a/m4/pthread.m4 b/m4/pthread.m4
> index 69866cb..2b00944 100644
> --- a/m4/pthread.m4
> +++ b/m4/pthread.m4
> @@ -5,25 +5,50 @@ dnl gives unlimited permission to copy and/or distribute it,
> dnl with or without modifications, as long as this notice is preserved.
> 
> AC_DEFUN([gl_PTHREAD_CHECK],
> -  [AC_CHECK_HEADERS_ONCE([pthread.h])
> +[
> +   AC_REQUIRE([gl_PTHREAD_DEFAULTS])
> +   AC_CHECK_HEADERS_ONCE([pthread.h])
> +   gl_CHECK_NEXT_HEADERS([pthread.h])
> +   if test $ac_cv_header_pthread_h = yes; then
> +     HAVE_PTHREAD_H=1
> +   else
> +     HAVE_PTHREAD_H=0
> +   fi
> +
> +   AC_CHECK_TYPES([pthread_t, pthread_spinlock_t])
> +   if test $ac_cv_type_pthread_t != yes; then
> +     HAVE_PTHREAD_T=0
> +   fi
> +   if test $ac_cv_type_pthread_spinlock_t != yes; then
> +     HAVE_PTHREAD_SPINLOCK_T=0
> +   fi
> +
> +   if test $ac_cv_header_pthread_h != yes ||
> +      test $ac_cv_type_pthread_t != yes ||
> +      test $ac_cv_type_pthread_spinlock_t != yes; then
> +     PTHREAD_H='pthread.h'
> +   fi
> 
>    LIB_PTHREAD=
> -   PTHREAD_H=
> -   if test "$ac_cv_header_pthread_h" = yes; then
> +   if test $ac_cv_header_pthread_h = yes; then
>      gl_saved_libs=$LIBS
>      AC_SEARCH_LIBS([pthread_create], [pthread],
>        [if test "$ac_cv_search_pthread_create" != "none required"; then
>           LIB_PTHREAD="$ac_cv_search_pthread_create"
>         fi])
>      LIBS="$gl_saved_libs"
> -   else
> -     AC_CHECK_TYPES([pthread_t])
> -     PTHREAD_H='pthread.h'
>    fi
> -
>    AC_SUBST([LIB_PTHREAD])
> -   AC_SUBST([PTHREAD_H])
> 
>    AC_REQUIRE([AC_C_INLINE])
>    AC_REQUIRE([AC_C_RESTRICT])
> ])
> +
> +AC_DEFUN([gl_PTHREAD_DEFAULTS],
> +[
> +  dnl Assume proper GNU behavior unless another module says otherwise.
> +  HAVE_PTHREAD_H=1;              AC_SUBST([HAVE_PTHREAD_H])
> +  HAVE_PTHREAD_T=1;              AC_SUBST([HAVE_PTHREAD_T])
> +  HAVE_PTHREAD_SPINLOCK_T=1;     AC_SUBST([HAVE_PTHREAD_SPINLOCK_T])
> +  PTHREAD_H='';                  AC_SUBST([PTHREAD_H])
> +])
> diff --git a/modules/pthread b/modules/pthread
> index 51d5dbb..6d06464 100644
> --- a/modules/pthread
> +++ b/modules/pthread
> @@ -18,9 +18,18 @@ BUILT_SOURCES += $(PTHREAD_H)
> # We need the following in order to create <pthread.h> when the system
> # doesn't have one that works with the given compiler.
> pthread.h: pthread.in.h
> -	$(AM_V_GEN)ln -f $(srcdir)/pthread.in.h $@ \
> -          || cp $(srcdir)/pthread.in.h $@
> -MOSTLYCLEANFILES += pthread.h
> +	$(AM_V_GEN)rm -f $@-t $@ && \
> +	{ echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
> +	  sed -e 's|@''HAVE_PTHREAD_H''@|$(HAVE_PTHREAD_H)|g' \
> +	      -e 's|@''INCLUDE_NEXT''@|$(INCLUDE_NEXT)|g' \
> +	      -e 's|@''PRAGMA_SYSTEM_HEADER''@|@PRAGMA_SYSTEM_HEADER@|g' \
> +	      -e 's|@''NEXT_PTHREAD_H''@|$(NEXT_PTHREAD_H)|g' \
> +	      -e 's|@''HAVE_PTHREAD_T''@|$(HAVE_PTHREAD_T)|g' \
> +	      -e 's|@''HAVE_PTHREAD_SPINLOCK_T''@|$(HAVE_PTHREAD_SPINLOCK_T)|g' \
> +	      < $(srcdir)/pthread.in.h; \
> +	} > $@-t && \
> +	mv $@-t $@
> +MOSTLYCLEANFILES += pthread.h pthread.h-t
> 
> Include:
> <pthread.h>

Cheers,
-- 
Gary V. Vaughan ([email-masked])