Discussion:
bug#7073: no pthread_spinlock_t on Mac OS 10.6.4
Paul Eggert
2010-09-20 06:21:03 UTC
Permalink
my system headers have no pthread_spinlock_t definition, but
gnulib sees /usr/include/pthread.h and uses that instead of generating it's own,
...
I don't know enough about pthreads to tell whether gnulib should paper over
the lack of spinlocks on Mac OS, or whether coreutils should be more careful
about using spinlocks without having tested for them first...
If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
doesn't have proper thread support. The simplest fix is for coreutils
to use the substitute pthread.h on MacOS. Does the following patch
work for you?

diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..121d84d 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,22 @@ dnl with or without modifications, as long as this notice is preserved.

AC_DEFUN([gl_PTHREAD_CHECK],
[AC_CHECK_HEADERS_ONCE([pthread.h])
+ AC_CHECK_TYPES([pthread_t])

LIB_PTHREAD=
- PTHREAD_H=
- 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'
+ PTHREAD_H='pthread.h'
+ if test "$ac_cv_header_pthread_h" = yes &&
+ test "$ac_cv_type_pthread_t" = yes; then
+ AC_CHECK_TYPE([pthread_spinlock_t])
+ if test "$ac_cv_type_pthread_spinlock_t" = yes; then
+ PTHREAD_H=
+ 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"
+ fi
fi

AC_SUBST([LIB_PTHREAD])
Pádraig Brady
2010-09-20 09:33:34 UTC
Permalink
Post by Paul Eggert
my system headers have no pthread_spinlock_t definition, but
gnulib sees /usr/include/pthread.h and uses that instead of generating it's own,
...
I don't know enough about pthreads to tell whether gnulib should paper over
the lack of spinlocks on Mac OS, or whether coreutils should be more careful
about using spinlocks without having tested for them first...
If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
doesn't have proper thread support. The simplest fix is for coreutils
to use the substitute pthread.h on MacOS. Does the following patch
work for you?
On a related note, I've been meaning to check
if mutexes in coreutils/sort are more stable
and/or fair to the system than spinlocks.
If we don't end up moving to mutexes everywhere,
perhaps we should do it for Mac OS always?

Also on the same point of more appropriate use
of system resources, perhaps we should cap the
number of cores used to perhaps 8 given results like:
Loading Image...;att=1;bug=6600
We would allow increasing up to the number of cores
available on the system by user specified values
(which currently only support decreasing from #cores).

cheers,
Pádraig.
Paul Eggert
2010-09-20 18:00:33 UTC
Permalink
Post by Pádraig Brady
On a related note, I've been meaning to check
if mutexes in coreutils/sort are more stable
and/or fair to the system than spinlocks.
They ought to be fairer, though I'd expect there to be a significant
performance price in some cases. I'm not sure what you mean
by "stable" (surely this has nothing to do with stable sorting :-).

It's a bit tricky, as some users prefer performance, others fairness,
and the default can't satisfy everybody.
Post by Pádraig Brady
perhaps we should cap the
http://debbugs.gnu.org/cgi/bugreport.cgi?msg=49;filename=sort-amd-24way.png;att=1;bug=6600
We would allow increasing up to the number of cores
available on the system by user specified values
(which currently only support decreasing from #cores).
That result depends on the particular benchmark and platform,
of course. Still, it might make sense to default to at most 8
for now, as you suggest; the documentation should say that that
default might change of course.

Another thought: the syntax for --parallel might be generalized
to allow for fancier heuristics, e.g.:

--parallel=50%

would mean to use at most half the processors. This particular
syntax would be similar to how --buffer-size acts.
Chen Guo
2010-09-20 21:38:12 UTC
Permalink
Hi all,

First regarding mutexes on Macs, I suppose a performance hit with mutexes beats
no performance at all with missing spinlocks. And regarding "take more work," I
believe spinlocks and mutexes were basically interchangeable in terms
of functionality
in our sort algorithm; the work probably will be little more than a
few #ifdefs and a
s/pthread_mutex_t/pthread_spinlock_t/

How difficult would it be to implement a basic spinlock in gnulib,
though? Perhaps we
can implement that and fall back to it on MacOS X?
Post by Paul Eggert
Post by Pádraig Brady
On a related note, I've been meaning to check
if mutexes in coreutils/sort are more stable
and/or fair to the system than spinlocks.
They ought to be fairer, though I'd expect there to be a significant
performance price in some cases.  I'm not sure what you mean
by "stable" (surely this has nothing to do with stable sorting :-).
An example of the stability issues is be the hyperthreaded processor issue.
For example, when I was sorting on an i7 (4 cores, 8 logical), sort time with
threads > 4 had very high variance with spinlocks, but not with mutexes.

I do believe this particular issue is handled; I remember capping threads
to the number of physical cores on the system. Of course, there might be
other nasties that we just don't know about yet with spinlocks.
Post by Paul Eggert
Post by Pádraig Brady
perhaps we should cap the
http://debbugs.gnu.org/cgi/bugreport.cgi?msg=49;filename=sort-amd-24way.png;att=1;bug=6600
We would allow increasing up to the number of cores
available on the system by user specified values
(which currently only support decreasing from #cores).
That result depends on the particular benchmark and platform,
of course.  Still, it might make sense to default to at most 8
for now, as you suggest; the documentation should say that that
default might change of course.
This is a good idea... I'm not sure how feasible this would be, but perhaps
we can benchmark some common server systems or some performance
outliers and define a custom max for these.
Paul Eggert
2010-09-20 23:38:07 UTC
Permalink
How difficult would it be to implement a basic spinlock in gnulib, though?
Portably? I'd think it'd be quite a pain, as it would require
figuring out this platform's atomic instructions, dealing with memory
barriers, and the like.
I suppose a performance hit with mutexes beats no performance at all
with missing spinlocks.
Yes.
And regarding "take more work," I believe spinlocks and mutexes were
basically interchangeable in terms of functionality in our sort
algorithm; the work probably will be little more than a few #ifdefs
and a s/pthread_mutex_t/pthread_spinlock_t/
Is that a reasonably-valid replacement in general, for code that uses
spin locks? If so, we should implement this inside gnulib's pthread
module. If not, it needs to be done inside coreutils, or perhaps
as a separate gnulib module.
Bruno Haible
2010-09-20 09:24:55 UTC
Permalink
Hi Paul,
Post by Paul Eggert
If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
doesn't have proper thread support.
Usually in applications mutexes are used, not spinlocks. The comments in sort.c
say:

"Note spin locks were seen to perform better than mutexes
as long as the number of threads is limited to the
number of processors."

In other words, spinlocks were chosen instead of mutexes only for performance
reasons.

So, instead of turning off the entire thread-based parallelization on MacOS X,
why not use mutexes instead of spinlocks on this platform?

Bruno
Paul Eggert
2010-09-20 17:32:27 UTC
Permalink
Post by Bruno Haible
why not use mutexes instead of spinlocks on this platform?
That might be a good thing to try, though it'd take more work.

Chen, what do you think? Here's the email that started this discussion:
http://lists.gnu.org/archive/html/bug-coreutils/2010-09/msg00065.html
Gary V. Vaughan
2010-09-20 02:43:43 UTC
Permalink
While testing my rewrite of gnulib's bootstrap script, I've been
unable to rebuild coreutils from git master. However, when I
reverted to a fresh checkout and used the repo bootstrap script,
the build still fails on my Mac:

../../src/sort.c:243: error: expected specifier-qualifier-list before 'pthread_spinlock_t'
../../src/sort.c: In function 'lock_node':
../../src/sort.c:3159: warning: implicit declaration of function 'pthread_spin_lock'
../../src/sort.c:3159: error: 'struct merge_node' has no member named 'lock'
../../src/sort.c: In function 'unlock_node':
../../src/sort.c:3167: warning: implicit declaration of function 'pthread_spin_unlock'
../../src/sort.c:3167: error: 'struct merge_node' has no member named 'lock'
../../src/sort.c: In function 'sortlines':
../../src/sort.c:3445: error: 'pthread_spinlock_t' undeclared (first use in this function)
../../src/sort.c:3445: error: (Each undeclared identifier is reported only once
../../src/sort.c:3445: error: for each function it appears in.)
../../src/sort.c:3445: error: expected ';' before 'lock'
../../src/sort.c:3446: warning: implicit declaration of function 'pthread_spin_init'
../../src/sort.c:3446: error: 'lock' undeclared (first use in this function)
../../src/sort.c:3448: warning: excess elements in struct initializer
../../src/sort.c:3448: warning: (near initialization for 'node')
../../src/sort.c: In function 'sort':
../../src/sort.c:3759: error: 'pthread_spinlock_t' undeclared (first use in this function)
../../src/sort.c:3759: error: expected ';' before 'lock'
../../src/sort.c:3760: error: 'lock' undeclared (first use in this function)
../../src/sort.c:3763: warning: excess elements in struct initializer
../../src/sort.c:3763: warning: (near initialization for 'node')

lib/config.h contains:

/* Define to 1 if you have the `pthread_atfork' function. */
#define HAVE_PTHREAD_ATFORK 1

/* Define to 1 if you have the <pthread.h> header file. */
#define HAVE_PTHREAD_H 1

/* Define if the <pthread.h> defines PTHREAD_MUTEX_RECURSIVE. */
#define HAVE_PTHREAD_MUTEX_RECURSIVE 1

/* Define if the POSIX multithreading library has read/write locks. */
#define HAVE_PTHREAD_RWLOCK 1

/* Define to 1 if the system has the type `pthread_t'. */
/* #undef HAVE_PTHREAD_T */

So, in this case, my system headers have no pthread_spinlock_t definition, but
gnulib sees /usr/include/pthread.h and uses that instead of generating it's own,
even though gnulib's pthread.h makes no allowance for lack of spinlocks when
HAVE_PTHREAD_T is undefined as in my case.

I don't know enough about pthreads to tell whether gnulib should paper over
the lack of spinlocks on Mac OS, or whether coreutils should be more careful
about using spinlocks without having tested for them first...

Cheers,
--
Gary V. Vaughan (***@gnu.org)
Gary V. Vaughan
2010-09-20 07:14:46 UTC
Permalink
Hi Paul,

Thanks for looking at this.
Post by Paul Eggert
my system headers have no pthread_spinlock_t definition, but
gnulib sees /usr/include/pthread.h and uses that instead of generating it's own,
...
I don't know enough about pthreads to tell whether gnulib should paper over
the lack of spinlocks on Mac OS, or whether coreutils should be more careful
about using spinlocks without having tested for them first...
If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
doesn't have proper thread support. The simplest fix is for coreutils
to use the substitute pthread.h on MacOS. Does the following patch
work for you?
It fixes the original spinlocks error I reported, but then the build falls over on lib/glthread/lock.c:

In file included from ../../lib/glthread/lock.h:93,
from ../../lib/glthread/lock.c:26:
./pthread.h:184: error: expected ')' before '*' token
./pthread.h:191: error: expected ')' before '*' token
./pthread.h:198: error: expected ')' before '*' token
./pthread.h:206: error: expected ')' before '*' token
../../lib/glthread/lock.c: In function 'glthread_recursive_lock_init_multithreaded':
../../lib/glthread/lock.c:321: warning: implicit declaration of function 'pthread_mutexattr_init'
../../lib/glthread/lock.c:324: warning: implicit declaration of function 'pthread_mutexattr_settype'
../../lib/glthread/lock.c:327: warning: implicit declaration of function 'pthread_mutexattr_destroy'

where lines 184, 191, 198 and 206 of lib/pthread.h each contain a use of pthread_spinlock_t.

lib/pthread.h does have a typedef for pthread_spinlock_t, but it is predicated on not having HAVE_PTHREAD_T defined (which I do):

; sed -n /HAVE_PTHREAD_T/,/endif/p lib/pthread.h
#ifndef HAVE_PTHREAD_T
typedef int pthread_t;
typedef int pthread_attr_t;
typedef int pthread_barrier_t;
typedef int pthread_barrierattr_t;
typedef int pthread_cond_t;
typedef int pthread_condattr_t;
typedef int pthread_key_t;
typedef int pthread_mutex_t;
typedef int pthread_mutexattr_t;
typedef int pthread_once_t;
typedef int pthread_rwlock_t;
typedef int pthread_rwlockattr_t;
typedef int pthread_spinlock_t;
#endif
; grep HAVE_PTHREAD_T lib/config.h
#define HAVE_PTHREAD_T 1
Post by Paul Eggert
diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..121d84d 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,22 @@ dnl with or without modifications, as long as this notice is preserved.
AC_DEFUN([gl_PTHREAD_CHECK],
[AC_CHECK_HEADERS_ONCE([pthread.h])
+ AC_CHECK_TYPES([pthread_t])
LIB_PTHREAD=
- PTHREAD_H=
- 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'
+ PTHREAD_H='pthread.h'
+ if test "$ac_cv_header_pthread_h" = yes &&
+ test "$ac_cv_type_pthread_t" = yes; then
+ AC_CHECK_TYPE([pthread_spinlock_t])
+ if test "$ac_cv_type_pthread_spinlock_t" = yes; then
+ PTHREAD_H=
+ 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"
+ fi
fi
AC_SUBST([LIB_PTHREAD])
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-20 17:30:17 UTC
Permalink
Ah, OK. Could you try the following patch instead? If you have a
similar problem with (say) pthread_rwlockattr_t, you can treat it just
like pthread_spinlock_t in lib/pthread.in.h and m4/pthread.m4; please
just let us know which types those are. I'd
rather not add compile-time checks for each dinky little pthread type
unless it's known to be needed, as 'configure' takes too long already.

diff --git a/lib/pthread.in.h b/lib/pthread.in.h
index 4dad22a..84cf913 100644
--- a/lib/pthread.in.h
+++ b/lib/pthread.in.h
@@ -40,6 +40,8 @@
typedef int pthread_once_t;
typedef int pthread_rwlock_t;
typedef int pthread_rwlockattr_t;
+#endif
+#ifndef HAVE_PTHREAD_SPINLOCK_T
typedef int pthread_spinlock_t;
#endif

diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..a2781eb 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,21 @@ dnl with or without modifications, as long as this notice is preserved.

AC_DEFUN([gl_PTHREAD_CHECK],
[AC_CHECK_HEADERS_ONCE([pthread.h])
+ gl_keep_pthread_h=$ac_cv_header_pthread_h
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t],
+ [],
+ [gl_keep_pthread_h=no])

+ PTHREAD_H='pthread.h'
LIB_PTHREAD=
- PTHREAD_H=
- if test "$ac_cv_header_pthread_h" = yes; then
+ if test "$gl_keep_pthread_h" = yes; then
+ PTHREAD_H=
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])
Gary V. Vaughan
2010-09-21 00:43:52 UTC
Permalink
Hi Paul,
Post by Paul Eggert
Ah, OK. Could you try the following patch instead?
The patch you attached allows compilation to complete on my mac, and produces a working (according to my very cursory testing!) sort binary.

I notice the following warnings though:

../../lib/glthread/lock.c: In function 'glthread_recursive_lock_init_multithreaded':
../../lib/glthread/lock.c:319: warning: implicit declaration of function 'pthread_mutexattr_init'
../../lib/glthread/lock.c:322: warning: implicit declaration of function 'pthread_mutexattr_settype'
../../lib/glthread/lock.c:325: warning: implicit declaration of function 'pthread_mutexattr_destroy'

Which is odd, because /usr/include/pthread.h contains (unguarded) prototypes for all those functions - so I assume the system pthread.h is either being mangled, or entirely skipped when glthread/lock.c includes the gnulib/pthread.h?

* time passes *

Hmm... looking at gnulib/pthread.h, am I right in assuming that threads are now effectively disabled by gnulib on the mac in favour of a selection of stub functions? In which case, I suppose those warnings indicate that glthread/lock.c may attempt to call unstubbed functions along some codepath and will then crash.
Post by Paul Eggert
diff --git a/lib/pthread.in.h b/lib/pthread.in.h
index 4dad22a..84cf913 100644
--- a/lib/pthread.in.h
+++ b/lib/pthread.in.h
@@ -40,6 +40,8 @@
typedef int pthread_once_t;
typedef int pthread_rwlock_t;
typedef int pthread_rwlockattr_t;
+#endif
+#ifndef HAVE_PTHREAD_SPINLOCK_T
typedef int pthread_spinlock_t;
#endif
diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..a2781eb 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,21 @@ dnl with or without modifications, as long as this notice is preserved.
AC_DEFUN([gl_PTHREAD_CHECK],
[AC_CHECK_HEADERS_ONCE([pthread.h])
+ gl_keep_pthread_h=$ac_cv_header_pthread_h
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t],
+ [],
+ [gl_keep_pthread_h=no])
+ PTHREAD_H='pthread.h'
LIB_PTHREAD=
- PTHREAD_H=
- if test "$ac_cv_header_pthread_h" = yes; then
+ if test "$gl_keep_pthread_h" = yes; then
+ PTHREAD_H=
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])
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-21 07:49:37 UTC
Permalink
Post by Gary V. Vaughan
Hmm... looking at gnulib/pthread.h, am I right in assuming that
threads are now effectively disabled by gnulib on the mac in favour
of a selection of stub functions? In which case, I suppose those
warnings indicate that glthread/lock.c may attempt to call unstubbed
functions along some codepath and will then crash.
Yes, that's right. The problem is that the gnulib i18n code includes
pthread.h for its own purposes, and wants primitives that our trivial
pthread replacement doesn't supply.

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.

The patch below has a chance of working only because the gnulib i18n
code doesn't need spinlocks: if it did, we'd need something fancier.
Perhaps someday there should be a way to tell the i18n code "Hey! This
app doesn't use threads so don't try to include <pthread.h> or link to
the thread library."

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


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>
Bruno Haible
2010-09-21 10:43:48 UTC
Permalink
Hi Paul,
The problem is that the gnulib i18n code includes pthread.h for its
own purposes
I wouldn't call it "the gnulib i18n code". The modules 'lock', 'tls',
etc. are needed by the modules 'strsignal', 'fstrcmp', and 'localename'.
Basically, every piece of code that wants to provide multithreaded
access to a global variable (may it be a registry or cache or anything)
needs locking.
and wants primitives that our trivial pthread replacement doesn't supply.
Sure, when gnulib provides a "replacement" of a system header, it should
not remove features from the system headers that other parties may rely
upon.
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t])
It would be wise to include <pthread.h> here. Not all systems define
these types in <sys/types.h>; some (e.g. mingw pthreads-win32) require
a #include <pthread.h>. This include is not part of AC_INCLUDES_DEFAULT.

Bruno
Paul Eggert
2010-09-21 17:23:10 UTC
Permalink
Post by Bruno Haible
The modules 'lock', 'tls',
etc. are needed by the modules 'strsignal', 'fstrcmp', and 'localename'.
Basically, every piece of code that wants to provide multithreaded
access to a global variable (may it be a registry or cache or anything)
needs locking.
Yes, but the problem is that coreutils does not need and does not want
that multithreaded access, and yet it has to build the multithreaded
support anyway. The runtime cost is probably small, but it introduces
dependencies and porting problems that are not worth the hassle. And
I worry that it may cause unreliability at runtime, on some platforms.

It'd be better if applications could say "I don't need gnulib to be
multithread-safe, and please don't bother with thread-safety"
when it incorporates library code from gnulib. Even 'sort', the only
coreutils program that is multithreaded, doesn't need the gnulib code
itself to be thread-safe.
Post by Bruno Haible
Post by Paul Eggert
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t])
It would be wise to include <pthread.h> here. Not all systems define
these types in <sys/types.h>; some (e.g. mingw pthreads-win32) require
a #include <pthread.h>. This include is not part of AC_INCLUDES_DEFAULT.
Thanks, I'll incorporate the following further patch in my next suggestion.
I don't think this'll affect Gary's problem.

--- old/m4/pthread.m4 2010-09-21 00:25:57.628643000 -0700
+++ new/m4/pthread.m4 2010-09-21 10:03:50.638754000 -0700
@@ -15,7 +15,11 @@ AC_DEFUN([gl_PTHREAD_CHECK],
HAVE_PTHREAD_H=0
fi

- AC_CHECK_TYPES([pthread_t, pthread_spinlock_t])
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t], [], [],
+ [AC_INCLUDES_DEFAULT[
+ #if HAVE_PTHREAD_H
+ #include <pthread.h>
+ #endif]])
if test $ac_cv_type_pthread_t != yes; then
HAVE_PTHREAD_T=0
fi
Bruno Haible
2010-09-21 17:40:43 UTC
Permalink
Post by Paul Eggert
the problem is that coreutils does not need and does not want
that multithreaded access, and yet it has to build the multithreaded
support anyway. ...
It'd be better if applications could say "I don't need gnulib to be
multithread-safe, and please don't bother with thread-safety"
You can do so by inserting
gl_use_threads_default=no
in your configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.

Users who install coreutils can do so by passing the option --disable-threads.

But beware: since some coreutils programs _are_ multithreaded (namely,
'sort'), this option can introduce bugs, if you don't control very carefully
which API you invoke while the concurrent threads are running.

Bruno
Paul Eggert
2010-09-21 19:03:07 UTC
Permalink
Gary, could you please also try the following patch to coreutils,
either by itself, or along with the updated gnulib patch? Thanks.

configure: default gnulib to not using threads

* configure.ac: Set gl_use_threads_default=no so that gnulib is
configured without threading. The only coreutils application that
uses threads (namely 'sort') does not invoke gnulib code in ways
that could lead to races. Setting this flag simplifies
configuration, making it less likely for builds to fail, and
perhaps preventing some runtime gotchas.
diff --git a/configure.ac b/configure.ac
index acd397e..15368f2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -35,6 +35,11 @@ AC_CONFIG_HEADERS([lib/config.h:lib/config.hin])
AM_INIT_AUTOMAKE([1.11.1 dist-xz color-tests parallel-tests])
AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.

+# The gnulib code need not be multithread-safe. Only 'sort' is
+# multithreaded, and its concurrent threads do not require gnulib's
+# multithread-aware code.
+gl_use_threads_default=no
+
AC_PROG_CC_STDC
AM_PROG_CC_C_O
AC_PROG_CPP
Gary V. Vaughan
2010-09-22 08:18:29 UTC
Permalink
Hi Paul,
Post by Paul Eggert
Gary, could you please also try the following patch to coreutils,
either by itself,
gcc -std=gnu99 -I. -I../../src -I../lib -I../../lib -g -O2 -MT sort.o -MD -MP -MF .deps/sort.Tpo -c -o sort.o ../../src/sort.c
../../src/sort.c:243: error: expected specifier-qualifier-list before 'pthread_spinlock_t'
../../src/sort.c: In function 'lock_node':
../../src/sort.c:3159: warning: implicit declaration of function 'pthread_spin_lock'
../../src/sort.c:3159: error: 'struct merge_node' has no member named 'lock'
../../src/sort.c: In function 'unlock_node':
../../src/sort.c:3167: warning: implicit declaration of function 'pthread_spin_unlock'
../../src/sort.c:3167: error: 'struct merge_node' has no member named 'lock'
../../src/sort.c: In function 'sortlines':
../../src/sort.c:3445: error: 'pthread_spinlock_t' undeclared (first use in this function)
../../src/sort.c:3445: error: (Each undeclared identifier is reported only once
../../src/sort.c:3445: error: for each function it appears in.)
../../src/sort.c:3445: error: expected ';' before 'lock'
../../src/sort.c:3446: warning: implicit declaration of function 'pthread_spin_init'
../../src/sort.c:3446: error: 'lock' undeclared (first use in this function)
../../src/sort.c:3448: warning: excess elements in struct initializer
../../src/sort.c:3448: warning: (near initialization for 'node')
../../src/sort.c: In function 'sort':
../../src/sort.c:3759: error: 'pthread_spinlock_t' undeclared (first use in this function)
../../src/sort.c:3759: error: expected ';' before 'lock'
../../src/sort.c:3760: error: 'lock' undeclared (first use in this function)
../../src/sort.c:3763: warning: excess elements in struct initializer
../../src/sort.c:3763: warning: (near initialization for 'node')
Post by Paul Eggert
or along with the updated gnulib patch?
Builds, and sort appears to be working, as it was in my last message on this thread.

Actually this patch seems to make no discernable difference to the behaviour of the build in both cases above.
Post by Paul Eggert
configure: default gnulib to not using threads
* configure.ac: Set gl_use_threads_default=no so that gnulib is
configured without threading. The only coreutils application that
uses threads (namely 'sort') does not invoke gnulib code in ways
that could lead to races. Setting this flag simplifies
configuration, making it less likely for builds to fail, and
perhaps preventing some runtime gotchas.
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-22 08:43:37 UTC
Permalink
Post by Gary V. Vaughan
Actually this patch seems to make no discernable difference to the behaviour of the build in both cases above.
That's odd, because adding "gl_use_threads_default=no" should cause
"configure" to default to --disable-threads behavior.

What is the difference between the outputs of
"./configure --disable-threads" and of
"./configure --enable-threads=posix"?
The former should behave as if the patch were installed,
and the latter as if it were not installed.

Assuming you have the gnulib patch that I just pushed,
you should see minor changes in the output of "configure",
as follows, where the "+" lines are with the patch
(or with --enable-threads=posix) and the "-" lines are
without the patch (or with --disable-threads).

@@ -333,7 +333,10 @@ checking whether getc_unlocked is declar
checking whether we are using the GNU C Library 2.1 or newer... yes
checking for wchar_t... yes
checking whether NULL can be used in arbitrary expressions... yes
-checking for multithread API to use... none
+checking whether imported symbols can be declared weak... yes
+checking for pthread.h... (cached) yes
+checking for pthread_kill in -lpthread... yes
+checking for multithread API to use... posix
checking for stdlib.h... (cached) yes
checking for GNU libc compatible malloc... yes
checking whether mbrtowc and mbstate_t are properly declared... (cached) yes
@@ -609,6 +612,7 @@ checking whether linkat handles trailing
checking whether locale.h conforms to POSIX:2001... yes
checking whether locale.h defines locale_t... yes
checking whether duplocale is declared without a macro... yes
+checking for pthread_rwlock_t... yes
checking whether lseek detects pipes... yes
checking for stdlib.h... (cached) yes
checking for GNU libc compatible malloc... (cached) yes
@@ -1010,6 +1014,7 @@ checking for wchar_t... (cached) yes
checking for wint_t... (cached) yes
checking for mmap... (cached) yes
checking for MAP_ANONYMOUS... yes
+checking for pthread_atfork... yes
checking for useconds_t... yes
checking whether usleep allows large arguments... yes
checking for a traditional french locale... (cached) fr_FR
@@ -1018,6 +1023,7 @@ checking for a traditional japanese loca
checking for a transitional chinese locale... (cached) zh_CN.GB18030
checking whether wctob works... yes
checking whether wctob is declared... (cached) yes
+checking for sched_yield in -lrt... yes
checking for library containing strerror... none required
checking for function prototypes... yes
checking for string.h... (cached) yes
Paul Eggert
2010-09-22 08:43:37 UTC
Permalink
Post by Gary V. Vaughan
Actually this patch seems to make no discernable difference to the behaviour of the build in both cases above.
That's odd, because adding "gl_use_threads_default=no" should cause
"configure" to default to --disable-threads behavior.

What is the difference between the outputs of
"./configure --disable-threads" and of
"./configure --enable-threads=posix"?
The former should behave as if the patch were installed,
and the latter as if it were not installed.

Assuming you have the gnulib patch that I just pushed,
you should see minor changes in the output of "configure",
as follows, where the "+" lines are with the patch
(or with --enable-threads=posix) and the "-" lines are
without the patch (or with --disable-threads).

@@ -333,7 +333,10 @@ checking whether getc_unlocked is declar
checking whether we are using the GNU C Library 2.1 or newer... yes
checking for wchar_t... yes
checking whether NULL can be used in arbitrary expressions... yes
-checking for multithread API to use... none
+checking whether imported symbols can be declared weak... yes
+checking for pthread.h... (cached) yes
+checking for pthread_kill in -lpthread... yes
+checking for multithread API to use... posix
checking for stdlib.h... (cached) yes
checking for GNU libc compatible malloc... yes
checking whether mbrtowc and mbstate_t are properly declared... (cached) yes
@@ -609,6 +612,7 @@ checking whether linkat handles trailing
checking whether locale.h conforms to POSIX:2001... yes
checking whether locale.h defines locale_t... yes
checking whether duplocale is declared without a macro... yes
+checking for pthread_rwlock_t... yes
checking whether lseek detects pipes... yes
checking for stdlib.h... (cached) yes
checking for GNU libc compatible malloc... (cached) yes
@@ -1010,6 +1014,7 @@ checking for wchar_t... (cached) yes
checking for wint_t... (cached) yes
checking for mmap... (cached) yes
checking for MAP_ANONYMOUS... yes
+checking for pthread_atfork... yes
checking for useconds_t... yes
checking whether usleep allows large arguments... yes
checking for a traditional french locale... (cached) fr_FR
@@ -1018,6 +1023,7 @@ checking for a traditional japanese loca
checking for a transitional chinese locale... (cached) zh_CN.GB18030
checking whether wctob works... yes
checking whether wctob is declared... (cached) yes
+checking for sched_yield in -lrt... yes
checking for library containing strerror... none required
checking for function prototypes... yes
checking for string.h... (cached) yes
Gary V. Vaughan
2010-09-22 08:18:29 UTC
Permalink
Hi Paul,
Post by Paul Eggert
Gary, could you please also try the following patch to coreutils,
either by itself,
gcc -std=gnu99 -I. -I../../src -I../lib -I../../lib -g -O2 -MT sort.o -MD -MP -MF .deps/sort.Tpo -c -o sort.o ../../src/sort.c
../../src/sort.c:243: error: expected specifier-qualifier-list before 'pthread_spinlock_t'
../../src/sort.c: In function 'lock_node':
../../src/sort.c:3159: warning: implicit declaration of function 'pthread_spin_lock'
../../src/sort.c:3159: error: 'struct merge_node' has no member named 'lock'
../../src/sort.c: In function 'unlock_node':
../../src/sort.c:3167: warning: implicit declaration of function 'pthread_spin_unlock'
../../src/sort.c:3167: error: 'struct merge_node' has no member named 'lock'
../../src/sort.c: In function 'sortlines':
../../src/sort.c:3445: error: 'pthread_spinlock_t' undeclared (first use in this function)
../../src/sort.c:3445: error: (Each undeclared identifier is reported only once
../../src/sort.c:3445: error: for each function it appears in.)
../../src/sort.c:3445: error: expected ';' before 'lock'
../../src/sort.c:3446: warning: implicit declaration of function 'pthread_spin_init'
../../src/sort.c:3446: error: 'lock' undeclared (first use in this function)
../../src/sort.c:3448: warning: excess elements in struct initializer
../../src/sort.c:3448: warning: (near initialization for 'node')
../../src/sort.c: In function 'sort':
../../src/sort.c:3759: error: 'pthread_spinlock_t' undeclared (first use in this function)
../../src/sort.c:3759: error: expected ';' before 'lock'
../../src/sort.c:3760: error: 'lock' undeclared (first use in this function)
../../src/sort.c:3763: warning: excess elements in struct initializer
../../src/sort.c:3763: warning: (near initialization for 'node')
Post by Paul Eggert
or along with the updated gnulib patch?
Builds, and sort appears to be working, as it was in my last message on this thread.

Actually this patch seems to make no discernable difference to the behaviour of the build in both cases above.
Post by Paul Eggert
configure: default gnulib to not using threads
* configure.ac: Set gl_use_threads_default=no so that gnulib is
configured without threading. The only coreutils application that
uses threads (namely 'sort') does not invoke gnulib code in ways
that could lead to races. Setting this flag simplifies
configuration, making it less likely for builds to fail, and
perhaps preventing some runtime gotchas.
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-21 19:03:07 UTC
Permalink
Gary, could you please also try the following patch to coreutils,
either by itself, or along with the updated gnulib patch? Thanks.

configure: default gnulib to not using threads

* configure.ac: Set gl_use_threads_default=no so that gnulib is
configured without threading. The only coreutils application that
uses threads (namely 'sort') does not invoke gnulib code in ways
that could lead to races. Setting this flag simplifies
configuration, making it less likely for builds to fail, and
perhaps preventing some runtime gotchas.
diff --git a/configure.ac b/configure.ac
index acd397e..15368f2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -35,6 +35,11 @@ AC_CONFIG_HEADERS([lib/config.h:lib/config.hin])
AM_INIT_AUTOMAKE([1.11.1 dist-xz color-tests parallel-tests])
AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.

+# The gnulib code need not be multithread-safe. Only 'sort' is
+# multithreaded, and its concurrent threads do not require gnulib's
+# multithread-aware code.
+gl_use_threads_default=no
+
AC_PROG_CC_STDC
AM_PROG_CC_C_O
AC_PROG_CPP
Paul Eggert
2010-09-21 18:56:56 UTC
Permalink
Post by Bruno Haible
You can do so by inserting
gl_use_threads_default=no
in your configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
Thanks, I didn't know that. I tried it, and found one minor glitch.
"configure" said "checking for multithread API to use... no"
which might imply to the casual reader that the resulting coreutils
would not use multiple threads anywhere. How about appending
"within threadlib" to that message?

Also, it'd be helpful to document gl_use_threads_default.

Perhaps the following patch?


diff --git a/m4/threadlib.m4 b/m4/threadlib.m4
index bff01bc..b6c1817 100644
--- a/m4/threadlib.m4
+++ b/m4/threadlib.m4
@@ -282,7 +282,7 @@ int main ()
fi
fi
fi
- AC_MSG_CHECKING([for multithread API to use])
+ AC_MSG_CHECKING([for multithread API to use within threadlib])
AC_MSG_RESULT([$gl_threads_api])
AC_SUBST([LIBTHREAD])
AC_SUBST([LTLIBTHREAD])
diff --git a/modules/gettext b/modules/gettext
index cab538e..787f237 100644
--- a/modules/gettext
+++ b/modules/gettext
@@ -38,6 +38,13 @@ gettext-h
havelib

configure.ac:
+# If your applications do not need gnulib to be multithread-safe,
+# either because they don't use threads or because they carefully
+# control which APIs are invoked while concurrent threads are running,
+# then you can avoid some build-time hassles and run-time overhead by
+# inserting:
+# gl_use_threads_default=no
+# early in configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
AM_GNU_GETTEXT([external])
AM_GNU_GETTEXT_VERSION([0.18.1])

diff --git a/modules/threadlib b/modules/threadlib
index 9e3438c..3e2226a 100644
--- a/modules/threadlib
+++ b/modules/threadlib
@@ -13,6 +13,13 @@ configure.ac-early:
gl_THREADLIB_EARLY

configure.ac:
+# If your applications do not need gnulib to be multithread-safe,
+# either because they don't use threads or because they carefully
+# control which APIs are invoked while concurrent threads are running,
+# then you can avoid some build-time hassles and run-time overhead by
+# inserting:
+# gl_use_threads_default=no
+# early in configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
gl_THREADLIB

Makefile.am:
Bruno Haible
2010-09-22 15:18:43 UTC
Permalink
Hi Paul,
Post by Bruno Haible
You can do so by inserting
gl_use_threads_default=no
in your configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
Thanks, I didn't know that. I tried it ...
Also, it'd be helpful to document gl_use_threads_default.
Well, I did not know about it either, before you asked ;-) But it's more
future-proof to ask coreutils to define a particular macro, rather than to
set a specific variable to a specific value. The same approach as we
already use in m4/ansi-c++.m4. I'm changing threadlib.m4 as below.
and found one minor glitch.
"configure" said "checking for multithread API to use... no"
which might imply to the casual reader that the resulting coreutils
would not use multiple threads anywhere. How about appending
"within threadlib" to that message?
- AC_MSG_CHECKING([for multithread API to use])
+ AC_MSG_CHECKING([for multithread API to use within threadlib])
It's peculiar to coreutils that it uses both 'threadlib' and 'pthread'.
For other packages, like gettext, the change that you suggest would be
misleading. Therefore I think it should better be done in a local
override of m4/threadlib.m4 in coreutils (via a tiny .diff file).
diff --git a/modules/gettext b/modules/gettext
index cab538e..787f237 100644
--- a/modules/gettext
+++ b/modules/gettext
@@ -38,6 +38,13 @@ gettext-h
havelib
+# If your applications do not need gnulib to be multithread-safe,
+# either because they don't use threads or because they carefully
+# control which APIs are invoked while concurrent threads are running,
+# then you can avoid some build-time hassles and run-time overhead by
+# gl_use_threads_default=no
+# early in configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
AM_GNU_GETTEXT([external])
AM_GNU_GETTEXT_VERSION([0.18.1])
I don't see what this has to do with the gettext module. This belongs only
in the threadlib module.

A general throught about the 'threadlib' vs. 'pthread' modules:
- The modules 'lock', 'tls', 'cond', 'thread', 'yield' attempt
broad portability. They work well on MacOS X, mingw (even without
pthreads-win32), Tru64, old Solaris, and others. But it does not
provide the POSIX API.
- The module 'pthread' provides the POSIX API, but don't work on older
Unix systems nor mingw.

I imagine a way to get the best of both is to change the API of
'lock', 'tls', 'cond', 'thread', 'yield' so that it provides a nearly
POSIX API. I say "nearly", because for the locks, it will make things
much easier to have separate functions for recursive locks that for
normal locks/mutexes. And a similar difference will probably remain
with pthread_key_t - this type does not provide enough information
when threads are turned off.

Would you (in coreutils, sort) be willing to use such an API that is
slightly different from POSIX, but much closer to POSIX than
'lock', 'tls', 'cond', 'thread', 'yield' are now?



2010-09-22 Bruno Haible <***@clisp.org>

threadlib: Allow the package to change the default to 'no'.
* m4/threadlib.m4 (gl_THREADLIB_EARLY_BODY): When
gl_THREADLIB_DEFAULT_NO is defined, change the default to 'no'.
Reported by Paul Eggert.

--- m4/threadlib.m4.orig Wed Sep 22 16:55:58 2010
+++ m4/threadlib.m4 Wed Sep 22 16:54:17 2010
@@ -1,4 +1,4 @@
-# threadlib.m4 serial 6 (gettext-0.18.2)
+# threadlib.m4 serial 7 (gettext-0.18.2)
dnl Copyright (C) 2005-2010 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -9,6 +9,11 @@
dnl gl_THREADLIB
dnl ------------
dnl Tests for a multithreading library to be used.
+dnl If the configure.ac contains a definition of the gl_THREADLIB_DEFAULT_NO
+dnl (it must be placed before the invocation of gl_THREADLIB_EARLY!), then the
+dnl default is 'no', otherwise it is system dependent. In both cases, the user
+dnl can change the choice through the options --enable-threads=choice or
+dnl --disable-threads.
dnl Defines at most one of the macros USE_POSIX_THREADS, USE_SOLARIS_THREADS,
dnl USE_PTH_THREADS, USE_WIN32_THREADS
dnl Sets the variables LIBTHREAD and LTLIBTHREAD to the linker options for use
@@ -44,10 +49,12 @@
[AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])],
[AC_REQUIRE([AC_GNU_SOURCE])])
dnl Check for multithreading.
- m4_divert_text([DEFAULTS], [gl_use_threads_default=])
+ m4_ifdef([gl_THREADLIB_DEFAULT_NO],
+ [m4_divert_text([DEFAULTS], [gl_use_threads_default=no])],
+ [m4_divert_text([DEFAULTS], [gl_use_threads_default=])])
AC_ARG_ENABLE([threads],
-AC_HELP_STRING([--enable-threads={posix|solaris|pth|win32}], [specify multithreading API])
-AC_HELP_STRING([--disable-threads], [build without multithread safety]),
+AC_HELP_STRING([--enable-threads={posix|solaris|pth|win32}], [specify multithreading API])m4_ifdef([gl_THREADLIB_DEFAULT_NO], [], [
+AC_HELP_STRING([--disable-threads], [build without multithread safety])]),
[gl_use_threads=$enableval],
[if test -n "$gl_use_threads_default"; then
gl_use_threads="$gl_use_threads_default"
Paul Eggert
2010-09-22 17:48:42 UTC
Permalink
Post by Bruno Haible
Would you (in coreutils, sort) be willing to use such an API that is
slightly different from POSIX, but much closer to POSIX than
'lock', 'tls', 'cond', 'thread', 'yield' are now?
Yes, that sounds like a better option, thanks! Two further thoughts.


First, for coreutils there's no need to implement the POSIX API for
recursive locks and for pthread_key_t, since coreutils doesn't need
those features. That is, instead of supplying a superset of a subset
of POSIX pthreads, it'd be fine to support just a subset of POSIX
pthreads.

If other applications need features where the POSIX API is hard to
support portably, we can worry about those features later (perhaps
never :-). I'd rather that coreutils didn't have to deal with any
hassles in supporting pthreads features it doesn't need. Perhaps
harder-to-support API features could be put into new modules, that the
pthread module would be a prerequisite for.

If it helps to simplify the work, here are the only pthread.h
features that coreutils needs:

pthread_t, pthread_cond_t, pthread_mutex_t, pthread_spinlock_t
pthread_create/join/destroy
pthread_spin_init/lock/unlock/destroy
pthread_cond_init/signal/wait/destroy
pthread_mutex_init/lock/unlock/destroy

All spinlocks are created with PTHREAD_PROCESS_PRIVATE; all other
objects are created with default attributes. Currently, on hosts
that have other pthreads features but not spinlocks, spinlocks
are emulated with mutexes.


Second, suppose we want to support coreutils' preference, of using
POSIX threads in sort, while not making the other gnulib functions be
thread-safe, because we know that 'sort' (and other coreutils apps, of
course) never invoke these gnulib functions from competing threads. I
assume gl_THREADLIB_DEFAULT_NO wouldn't be enough to support this
need, because it would shut off all threading, even in 'sort'. So
it'd be nice to have a way to support this need, too.
Assaf Gordon
2018-10-30 05:35:29 UTC
Permalink
(triaging old bugs)

Hello,

This thread ( https://bugs.gnu.org/7073 )
starts with build error on Mac OS X due to pthread related issues.

It then deals with this (already commited) gnulib change:
=====
2010-09-22 Bruno Haible <***@clisp.org>
threadlib: Allow the package to change the default to 'no'.
* m4/threadlib.m4 (gl_THREADLIB_EARLY_BODY): When
gl_THREADLIB_DEFAULT_NO is defined, change the default to 'no'.
=====
Post by Paul Eggert
Post by Bruno Haible
Would you (in coreutils, sort) be willing to use such an API that is
slightly different from POSIX, but much closer to POSIX than
'lock', 'tls', 'cond', 'thread', 'yield' are now?
Yes, that sounds like a better option, thanks! Two further thoughts.
Can this bug be closed?
(at least - I believe newer coreutils builds fine on newer Mac OS X)

thanks!
- assaf
Bruno Haible
2018-10-30 09:29:30 UTC
Permalink
Post by Assaf Gordon
Can this bug be closed?
(at least - I believe newer coreutils builds fine on newer Mac OS X)
Yes. In my understanding, there is no issue between coreutils and
gnulib's threadlib and pthread modules any more.

Bruno

Paul Eggert
2010-09-22 17:48:42 UTC
Permalink
Post by Bruno Haible
Would you (in coreutils, sort) be willing to use such an API that is
slightly different from POSIX, but much closer to POSIX than
'lock', 'tls', 'cond', 'thread', 'yield' are now?
Yes, that sounds like a better option, thanks! Two further thoughts.


First, for coreutils there's no need to implement the POSIX API for
recursive locks and for pthread_key_t, since coreutils doesn't need
those features. That is, instead of supplying a superset of a subset
of POSIX pthreads, it'd be fine to support just a subset of POSIX
pthreads.

If other applications need features where the POSIX API is hard to
support portably, we can worry about those features later (perhaps
never :-). I'd rather that coreutils didn't have to deal with any
hassles in supporting pthreads features it doesn't need. Perhaps
harder-to-support API features could be put into new modules, that the
pthread module would be a prerequisite for.

If it helps to simplify the work, here are the only pthread.h
features that coreutils needs:

pthread_t, pthread_cond_t, pthread_mutex_t, pthread_spinlock_t
pthread_create/join/destroy
pthread_spin_init/lock/unlock/destroy
pthread_cond_init/signal/wait/destroy
pthread_mutex_init/lock/unlock/destroy

All spinlocks are created with PTHREAD_PROCESS_PRIVATE; all other
objects are created with default attributes. Currently, on hosts
that have other pthreads features but not spinlocks, spinlocks
are emulated with mutexes.


Second, suppose we want to support coreutils' preference, of using
POSIX threads in sort, while not making the other gnulib functions be
thread-safe, because we know that 'sort' (and other coreutils apps, of
course) never invoke these gnulib functions from competing threads. I
assume gl_THREADLIB_DEFAULT_NO wouldn't be enough to support this
need, because it would shut off all threading, even in 'sort'. So
it'd be nice to have a way to support this need, too.
Bruno Haible
2010-09-22 15:18:43 UTC
Permalink
Hi Paul,
Post by Bruno Haible
You can do so by inserting
gl_use_threads_default=no
in your configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
Thanks, I didn't know that. I tried it ...
Also, it'd be helpful to document gl_use_threads_default.
Well, I did not know about it either, before you asked ;-) But it's more
future-proof to ask coreutils to define a particular macro, rather than to
set a specific variable to a specific value. The same approach as we
already use in m4/ansi-c++.m4. I'm changing threadlib.m4 as below.
and found one minor glitch.
"configure" said "checking for multithread API to use... no"
which might imply to the casual reader that the resulting coreutils
would not use multiple threads anywhere. How about appending
"within threadlib" to that message?
- AC_MSG_CHECKING([for multithread API to use])
+ AC_MSG_CHECKING([for multithread API to use within threadlib])
It's peculiar to coreutils that it uses both 'threadlib' and 'pthread'.
For other packages, like gettext, the change that you suggest would be
misleading. Therefore I think it should better be done in a local
override of m4/threadlib.m4 in coreutils (via a tiny .diff file).
diff --git a/modules/gettext b/modules/gettext
index cab538e..787f237 100644
--- a/modules/gettext
+++ b/modules/gettext
@@ -38,6 +38,13 @@ gettext-h
havelib
+# If your applications do not need gnulib to be multithread-safe,
+# either because they don't use threads or because they carefully
+# control which APIs are invoked while concurrent threads are running,
+# then you can avoid some build-time hassles and run-time overhead by
+# gl_use_threads_default=no
+# early in configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
AM_GNU_GETTEXT([external])
AM_GNU_GETTEXT_VERSION([0.18.1])
I don't see what this has to do with the gettext module. This belongs only
in the threadlib module.

A general throught about the 'threadlib' vs. 'pthread' modules:
- The modules 'lock', 'tls', 'cond', 'thread', 'yield' attempt
broad portability. They work well on MacOS X, mingw (even without
pthreads-win32), Tru64, old Solaris, and others. But it does not
provide the POSIX API.
- The module 'pthread' provides the POSIX API, but don't work on older
Unix systems nor mingw.

I imagine a way to get the best of both is to change the API of
'lock', 'tls', 'cond', 'thread', 'yield' so that it provides a nearly
POSIX API. I say "nearly", because for the locks, it will make things
much easier to have separate functions for recursive locks that for
normal locks/mutexes. And a similar difference will probably remain
with pthread_key_t - this type does not provide enough information
when threads are turned off.

Would you (in coreutils, sort) be willing to use such an API that is
slightly different from POSIX, but much closer to POSIX than
'lock', 'tls', 'cond', 'thread', 'yield' are now?



2010-09-22 Bruno Haible <***@clisp.org>

threadlib: Allow the package to change the default to 'no'.
* m4/threadlib.m4 (gl_THREADLIB_EARLY_BODY): When
gl_THREADLIB_DEFAULT_NO is defined, change the default to 'no'.
Reported by Paul Eggert.

--- m4/threadlib.m4.orig Wed Sep 22 16:55:58 2010
+++ m4/threadlib.m4 Wed Sep 22 16:54:17 2010
@@ -1,4 +1,4 @@
-# threadlib.m4 serial 6 (gettext-0.18.2)
+# threadlib.m4 serial 7 (gettext-0.18.2)
dnl Copyright (C) 2005-2010 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -9,6 +9,11 @@
dnl gl_THREADLIB
dnl ------------
dnl Tests for a multithreading library to be used.
+dnl If the configure.ac contains a definition of the gl_THREADLIB_DEFAULT_NO
+dnl (it must be placed before the invocation of gl_THREADLIB_EARLY!), then the
+dnl default is 'no', otherwise it is system dependent. In both cases, the user
+dnl can change the choice through the options --enable-threads=choice or
+dnl --disable-threads.
dnl Defines at most one of the macros USE_POSIX_THREADS, USE_SOLARIS_THREADS,
dnl USE_PTH_THREADS, USE_WIN32_THREADS
dnl Sets the variables LIBTHREAD and LTLIBTHREAD to the linker options for use
@@ -44,10 +49,12 @@
[AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])],
[AC_REQUIRE([AC_GNU_SOURCE])])
dnl Check for multithreading.
- m4_divert_text([DEFAULTS], [gl_use_threads_default=])
+ m4_ifdef([gl_THREADLIB_DEFAULT_NO],
+ [m4_divert_text([DEFAULTS], [gl_use_threads_default=no])],
+ [m4_divert_text([DEFAULTS], [gl_use_threads_default=])])
AC_ARG_ENABLE([threads],
-AC_HELP_STRING([--enable-threads={posix|solaris|pth|win32}], [specify multithreading API])
-AC_HELP_STRING([--disable-threads], [build without multithread safety]),
+AC_HELP_STRING([--enable-threads={posix|solaris|pth|win32}], [specify multithreading API])m4_ifdef([gl_THREADLIB_DEFAULT_NO], [], [
+AC_HELP_STRING([--disable-threads], [build without multithread safety])]),
[gl_use_threads=$enableval],
[if test -n "$gl_use_threads_default"; then
gl_use_threads="$gl_use_threads_default"
Paul Eggert
2010-09-21 18:56:56 UTC
Permalink
Post by Bruno Haible
You can do so by inserting
gl_use_threads_default=no
in your configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
Thanks, I didn't know that. I tried it, and found one minor glitch.
"configure" said "checking for multithread API to use... no"
which might imply to the casual reader that the resulting coreutils
would not use multiple threads anywhere. How about appending
"within threadlib" to that message?

Also, it'd be helpful to document gl_use_threads_default.

Perhaps the following patch?


diff --git a/m4/threadlib.m4 b/m4/threadlib.m4
index bff01bc..b6c1817 100644
--- a/m4/threadlib.m4
+++ b/m4/threadlib.m4
@@ -282,7 +282,7 @@ int main ()
fi
fi
fi
- AC_MSG_CHECKING([for multithread API to use])
+ AC_MSG_CHECKING([for multithread API to use within threadlib])
AC_MSG_RESULT([$gl_threads_api])
AC_SUBST([LIBTHREAD])
AC_SUBST([LTLIBTHREAD])
diff --git a/modules/gettext b/modules/gettext
index cab538e..787f237 100644
--- a/modules/gettext
+++ b/modules/gettext
@@ -38,6 +38,13 @@ gettext-h
havelib

configure.ac:
+# If your applications do not need gnulib to be multithread-safe,
+# either because they don't use threads or because they carefully
+# control which APIs are invoked while concurrent threads are running,
+# then you can avoid some build-time hassles and run-time overhead by
+# inserting:
+# gl_use_threads_default=no
+# early in configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
AM_GNU_GETTEXT([external])
AM_GNU_GETTEXT_VERSION([0.18.1])

diff --git a/modules/threadlib b/modules/threadlib
index 9e3438c..3e2226a 100644
--- a/modules/threadlib
+++ b/modules/threadlib
@@ -13,6 +13,13 @@ configure.ac-early:
gl_THREADLIB_EARLY

configure.ac:
+# If your applications do not need gnulib to be multithread-safe,
+# either because they don't use threads or because they carefully
+# control which APIs are invoked while concurrent threads are running,
+# then you can avoid some build-time hassles and run-time overhead by
+# inserting:
+# gl_use_threads_default=no
+# early in configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.
gl_THREADLIB

Makefile.am:
Bruno Haible
2010-09-21 17:40:43 UTC
Permalink
Post by Paul Eggert
the problem is that coreutils does not need and does not want
that multithreaded access, and yet it has to build the multithreaded
support anyway. ...
It'd be better if applications could say "I don't need gnulib to be
multithread-safe, and please don't bother with thread-safety"
You can do so by inserting
gl_use_threads_default=no
in your configure.ac, before the invocations of gl_INIT_EARLY and gl_INIT.

Users who install coreutils can do so by passing the option --disable-threads.

But beware: since some coreutils programs _are_ multithreaded (namely,
'sort'), this option can introduce bugs, if you don't control very carefully
which API you invoke while the concurrent threads are running.

Bruno
Paul Eggert
2010-09-21 17:23:10 UTC
Permalink
Post by Bruno Haible
The modules 'lock', 'tls',
etc. are needed by the modules 'strsignal', 'fstrcmp', and 'localename'.
Basically, every piece of code that wants to provide multithreaded
access to a global variable (may it be a registry or cache or anything)
needs locking.
Yes, but the problem is that coreutils does not need and does not want
that multithreaded access, and yet it has to build the multithreaded
support anyway. The runtime cost is probably small, but it introduces
dependencies and porting problems that are not worth the hassle. And
I worry that it may cause unreliability at runtime, on some platforms.

It'd be better if applications could say "I don't need gnulib to be
multithread-safe, and please don't bother with thread-safety"
when it incorporates library code from gnulib. Even 'sort', the only
coreutils program that is multithreaded, doesn't need the gnulib code
itself to be thread-safe.
Post by Bruno Haible
Post by Paul Eggert
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t])
It would be wise to include <pthread.h> here. Not all systems define
these types in <sys/types.h>; some (e.g. mingw pthreads-win32) require
a #include <pthread.h>. This include is not part of AC_INCLUDES_DEFAULT.
Thanks, I'll incorporate the following further patch in my next suggestion.
I don't think this'll affect Gary's problem.

--- old/m4/pthread.m4 2010-09-21 00:25:57.628643000 -0700
+++ new/m4/pthread.m4 2010-09-21 10:03:50.638754000 -0700
@@ -15,7 +15,11 @@ AC_DEFUN([gl_PTHREAD_CHECK],
HAVE_PTHREAD_H=0
fi

- AC_CHECK_TYPES([pthread_t, pthread_spinlock_t])
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t], [], [],
+ [AC_INCLUDES_DEFAULT[
+ #if HAVE_PTHREAD_H
+ #include <pthread.h>
+ #endif]])
if test $ac_cv_type_pthread_t != yes; then
HAVE_PTHREAD_T=0
fi
Bruno Haible
2010-09-21 10:43:48 UTC
Permalink
Hi Paul,
The problem is that the gnulib i18n code includes pthread.h for its
own purposes
I wouldn't call it "the gnulib i18n code". The modules 'lock', 'tls',
etc. are needed by the modules 'strsignal', 'fstrcmp', and 'localename'.
Basically, every piece of code that wants to provide multithreaded
access to a global variable (may it be a registry or cache or anything)
needs locking.
and wants primitives that our trivial pthread replacement doesn't supply.
Sure, when gnulib provides a "replacement" of a system header, it should
not remove features from the system headers that other parties may rely
upon.
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t])
It would be wise to include <pthread.h> here. Not all systems define
these types in <sys/types.h>; some (e.g. mingw pthreads-win32) require
a #include <pthread.h>. This include is not part of AC_INCLUDES_DEFAULT.

Bruno
Paul Eggert
2010-09-22 05:33:42 UTC
Permalink
But the patch causes a bizarre and apparently unrelated compilation
; 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'
I don't see how the patch could do that, unless sigprocmask.c
includes pthread.h somehow. Can you verify whether that happens,
by using gcc -E? If so, which file is the immediate includer
of pthread.h? And if not, what's the difference beween
the output of gcc -E sigprocmask.c, with and without the patch?

Also, could you please try the simpler patch here instead?

http://lists.gnu.org/archive/html/bug-coreutils/2010-09/msg00080.html
Gary V. Vaughan
2010-09-22 07:46:57 UTC
Permalink
Hi Paul,

Thanks for the swift response.
Post by Paul Eggert
But the patch causes a bizarre and apparently unrelated compilation
; 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'
I don't see how the patch could do that, unless sigprocmask.c
includes pthread.h somehow. Can you verify whether that happens,
by using gcc -E? If so, which file is the immediate includer
of pthread.h? And if not, what's the difference beween
the output of gcc -E sigprocmask.c, with and without the patch?
You're right. I guess my working directory had gotten out of synch with
something. After a `git reset --hard master && git clean -d -x -f', and
applying the patch again freshly, it works perfectly well, and without
the problems of undefined symbols like the previous patch.

Sorry for the noise.
Post by Paul Eggert
Also, could you please try the simpler patch here instead?
http://lists.gnu.org/archive/html/bug-coreutils/2010-09/msg00080.html
I'll reply in that thread when I've had time to try it.

Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-22 08:35:05 UTC
Permalink
Post by Gary V. Vaughan
After a `git reset --hard master && git clean -d -x -f', and
applying the patch again freshly, it works perfectly well, and without
the problems of undefined symbols like the previous patch.
OK, thanks, I pushed that patch, here:

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=66cc02681886cc3da705d0efb7a30a54bc8ce6b4
Paul Eggert
2010-09-22 08:35:05 UTC
Permalink
Post by Gary V. Vaughan
After a `git reset --hard master && git clean -d -x -f', and
applying the patch again freshly, it works perfectly well, and without
the problems of undefined symbols like the previous patch.
OK, thanks, I pushed that patch, here:

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=66cc02681886cc3da705d0efb7a30a54bc8ce6b4
Gary V. Vaughan
2010-09-22 07:46:57 UTC
Permalink
Hi Paul,

Thanks for the swift response.
Post by Paul Eggert
But the patch causes a bizarre and apparently unrelated compilation
; 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'
I don't see how the patch could do that, unless sigprocmask.c
includes pthread.h somehow. Can you verify whether that happens,
by using gcc -E? If so, which file is the immediate includer
of pthread.h? And if not, what's the difference beween
the output of gcc -E sigprocmask.c, with and without the patch?
You're right. I guess my working directory had gotten out of synch with
something. After a `git reset --hard master && git clean -d -x -f', and
applying the patch again freshly, it works perfectly well, and without
the problems of undefined symbols like the previous patch.

Sorry for the noise.
Post by Paul Eggert
Also, could you please try the simpler patch here instead?
http://lists.gnu.org/archive/html/bug-coreutils/2010-09/msg00080.html
I'll reply in that thread when I've had time to try it.

Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-22 05:33:42 UTC
Permalink
But the patch causes a bizarre and apparently unrelated compilation
; 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'
I don't see how the patch could do that, unless sigprocmask.c
includes pthread.h somehow. Can you verify whether that happens,
by using gcc -E? If so, which file is the immediate includer
of pthread.h? And if not, what's the difference beween
the output of gcc -E sigprocmask.c, with and without the patch?

Also, could you please try the simpler patch here instead?

http://lists.gnu.org/archive/html/bug-coreutils/2010-09/msg00080.html
Gary V. Vaughan
2010-09-22 05:11:11 UTC
Permalink
Hi Paul,
Post by Paul Eggert
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?
Post by Paul Eggert
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.
Post by Paul Eggert
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
+#endif
+
+/* The include_next requires a split double-inclusion guard. */
+#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
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
+
/* 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
+
+
+/* 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
-MOSTLYCLEANFILES += pthread.h
+ { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
+ < $(srcdir)/pthread.in.h; \
+MOSTLYCLEANFILES += pthread.h pthread.h-t
<pthread.h>
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Gary V. Vaughan
2010-09-22 05:11:11 UTC
Permalink
Hi Paul,
Post by Paul Eggert
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?
Post by Paul Eggert
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.
Post by Paul Eggert
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
+#endif
+
+/* The include_next requires a split double-inclusion guard. */
+#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
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
+
/* 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
+
+
+/* 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
-MOSTLYCLEANFILES += pthread.h
+ { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
+ < $(srcdir)/pthread.in.h; \
+MOSTLYCLEANFILES += pthread.h pthread.h-t
<pthread.h>
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-21 07:49:37 UTC
Permalink
Post by Gary V. Vaughan
Hmm... looking at gnulib/pthread.h, am I right in assuming that
threads are now effectively disabled by gnulib on the mac in favour
of a selection of stub functions? In which case, I suppose those
warnings indicate that glthread/lock.c may attempt to call unstubbed
functions along some codepath and will then crash.
Yes, that's right. The problem is that the gnulib i18n code includes
pthread.h for its own purposes, and wants primitives that our trivial
pthread replacement doesn't supply.

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.

The patch below has a chance of working only because the gnulib i18n
code doesn't need spinlocks: if it did, we'd need something fancier.
Perhaps someday there should be a way to tell the i18n code "Hey! This
app doesn't use threads so don't try to include <pthread.h> or link to
the thread library."

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


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>
Gary V. Vaughan
2010-09-21 00:43:52 UTC
Permalink
Hi Paul,
Post by Paul Eggert
Ah, OK. Could you try the following patch instead?
The patch you attached allows compilation to complete on my mac, and produces a working (according to my very cursory testing!) sort binary.

I notice the following warnings though:

../../lib/glthread/lock.c: In function 'glthread_recursive_lock_init_multithreaded':
../../lib/glthread/lock.c:319: warning: implicit declaration of function 'pthread_mutexattr_init'
../../lib/glthread/lock.c:322: warning: implicit declaration of function 'pthread_mutexattr_settype'
../../lib/glthread/lock.c:325: warning: implicit declaration of function 'pthread_mutexattr_destroy'

Which is odd, because /usr/include/pthread.h contains (unguarded) prototypes for all those functions - so I assume the system pthread.h is either being mangled, or entirely skipped when glthread/lock.c includes the gnulib/pthread.h?

* time passes *

Hmm... looking at gnulib/pthread.h, am I right in assuming that threads are now effectively disabled by gnulib on the mac in favour of a selection of stub functions? In which case, I suppose those warnings indicate that glthread/lock.c may attempt to call unstubbed functions along some codepath and will then crash.
Post by Paul Eggert
diff --git a/lib/pthread.in.h b/lib/pthread.in.h
index 4dad22a..84cf913 100644
--- a/lib/pthread.in.h
+++ b/lib/pthread.in.h
@@ -40,6 +40,8 @@
typedef int pthread_once_t;
typedef int pthread_rwlock_t;
typedef int pthread_rwlockattr_t;
+#endif
+#ifndef HAVE_PTHREAD_SPINLOCK_T
typedef int pthread_spinlock_t;
#endif
diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..a2781eb 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,21 @@ dnl with or without modifications, as long as this notice is preserved.
AC_DEFUN([gl_PTHREAD_CHECK],
[AC_CHECK_HEADERS_ONCE([pthread.h])
+ gl_keep_pthread_h=$ac_cv_header_pthread_h
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t],
+ [],
+ [gl_keep_pthread_h=no])
+ PTHREAD_H='pthread.h'
LIB_PTHREAD=
- PTHREAD_H=
- if test "$ac_cv_header_pthread_h" = yes; then
+ if test "$gl_keep_pthread_h" = yes; then
+ PTHREAD_H=
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])
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Paul Eggert
2010-09-20 17:30:17 UTC
Permalink
Ah, OK. Could you try the following patch instead? If you have a
similar problem with (say) pthread_rwlockattr_t, you can treat it just
like pthread_spinlock_t in lib/pthread.in.h and m4/pthread.m4; please
just let us know which types those are. I'd
rather not add compile-time checks for each dinky little pthread type
unless it's known to be needed, as 'configure' takes too long already.

diff --git a/lib/pthread.in.h b/lib/pthread.in.h
index 4dad22a..84cf913 100644
--- a/lib/pthread.in.h
+++ b/lib/pthread.in.h
@@ -40,6 +40,8 @@
typedef int pthread_once_t;
typedef int pthread_rwlock_t;
typedef int pthread_rwlockattr_t;
+#endif
+#ifndef HAVE_PTHREAD_SPINLOCK_T
typedef int pthread_spinlock_t;
#endif

diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..a2781eb 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,21 @@ dnl with or without modifications, as long as this notice is preserved.

AC_DEFUN([gl_PTHREAD_CHECK],
[AC_CHECK_HEADERS_ONCE([pthread.h])
+ gl_keep_pthread_h=$ac_cv_header_pthread_h
+ AC_CHECK_TYPES([pthread_t, pthread_spinlock_t],
+ [],
+ [gl_keep_pthread_h=no])

+ PTHREAD_H='pthread.h'
LIB_PTHREAD=
- PTHREAD_H=
- if test "$ac_cv_header_pthread_h" = yes; then
+ if test "$gl_keep_pthread_h" = yes; then
+ PTHREAD_H=
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])
Gary V. Vaughan
2010-09-20 07:14:46 UTC
Permalink
Hi Paul,

Thanks for looking at this.
Post by Paul Eggert
my system headers have no pthread_spinlock_t definition, but
gnulib sees /usr/include/pthread.h and uses that instead of generating it's own,
...
I don't know enough about pthreads to tell whether gnulib should paper over
the lack of spinlocks on Mac OS, or whether coreutils should be more careful
about using spinlocks without having tested for them first...
If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
doesn't have proper thread support. The simplest fix is for coreutils
to use the substitute pthread.h on MacOS. Does the following patch
work for you?
It fixes the original spinlocks error I reported, but then the build falls over on lib/glthread/lock.c:

In file included from ../../lib/glthread/lock.h:93,
from ../../lib/glthread/lock.c:26:
./pthread.h:184: error: expected ')' before '*' token
./pthread.h:191: error: expected ')' before '*' token
./pthread.h:198: error: expected ')' before '*' token
./pthread.h:206: error: expected ')' before '*' token
../../lib/glthread/lock.c: In function 'glthread_recursive_lock_init_multithreaded':
../../lib/glthread/lock.c:321: warning: implicit declaration of function 'pthread_mutexattr_init'
../../lib/glthread/lock.c:324: warning: implicit declaration of function 'pthread_mutexattr_settype'
../../lib/glthread/lock.c:327: warning: implicit declaration of function 'pthread_mutexattr_destroy'

where lines 184, 191, 198 and 206 of lib/pthread.h each contain a use of pthread_spinlock_t.

lib/pthread.h does have a typedef for pthread_spinlock_t, but it is predicated on not having HAVE_PTHREAD_T defined (which I do):

; sed -n /HAVE_PTHREAD_T/,/endif/p lib/pthread.h
#ifndef HAVE_PTHREAD_T
typedef int pthread_t;
typedef int pthread_attr_t;
typedef int pthread_barrier_t;
typedef int pthread_barrierattr_t;
typedef int pthread_cond_t;
typedef int pthread_condattr_t;
typedef int pthread_key_t;
typedef int pthread_mutex_t;
typedef int pthread_mutexattr_t;
typedef int pthread_once_t;
typedef int pthread_rwlock_t;
typedef int pthread_rwlockattr_t;
typedef int pthread_spinlock_t;
#endif
; grep HAVE_PTHREAD_T lib/config.h
#define HAVE_PTHREAD_T 1
Post by Paul Eggert
diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..121d84d 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,22 @@ dnl with or without modifications, as long as this notice is preserved.
AC_DEFUN([gl_PTHREAD_CHECK],
[AC_CHECK_HEADERS_ONCE([pthread.h])
+ AC_CHECK_TYPES([pthread_t])
LIB_PTHREAD=
- PTHREAD_H=
- 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'
+ PTHREAD_H='pthread.h'
+ if test "$ac_cv_header_pthread_h" = yes &&
+ test "$ac_cv_type_pthread_t" = yes; then
+ AC_CHECK_TYPE([pthread_spinlock_t])
+ if test "$ac_cv_type_pthread_spinlock_t" = yes; then
+ PTHREAD_H=
+ 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"
+ fi
fi
AC_SUBST([LIB_PTHREAD])
Cheers,
--
Gary V. Vaughan (***@gnu.org)
Loading...