GNOME Bugzilla – Bug 659866
pthread_rwlock_t requires defined __USE_UNIX98 || defined __USE_XOPEN2K
Last modified: 2011-10-03 02:33:36 UTC
On glibc 2.14 at least, we have in pthreadtypes.h: #if defined __USE_UNIX98 || defined __USE_XOPEN2K /* Data structure for read-write lock variable handling. The structure of the attribute type is not exposed on purpose. */ typedef union { ... } pthread_rwlock_t; #endif This means that anyone using G_RW_LOCK in their program needs a #define that will enable this, like _XOPEN_SOURCE 500 or GNU_SOURCE. We can temporarily make one of these #defines before we include pthread.h, but this will fail if anyone happens to include pthread.h before they include glib.h. But maybe we can punt on that.
This is a regression introduced by 3d4102776e59e748ee8b6e4d456a06a33593f308
I don't think it can be a regression, since GRWLock didn't even exist before that commit. One easy way out here might be to simply document the requirements on include order of glib.h vs pthread.h... But we certainly need a configure check for pthread_rwlock_t if we want to rely on it in this fashion.
One way forward that was explored on irc is to bring back the sizeof configure checks and avoid exposing the pthread_rwlock_t type in the headers
(In reply to comment #2) > I don't think it can be a regression, since GRWLock didn't even exist before > that commit. The regression is that other components and apps fail to compile against glib master, whereas they did before. All they're doing is including <gthread.h>. Having things compile is good.
I've now added a (disabled) testcase that shows the problem: glib/tests/include.c
*** Bug 660561 has been marked as a duplicate of this bug. ***
Created attachment 198053 [details] [review] GThread posix: switch to Windows ABI Modify the POSIX implementation of the synchronisation primatives to use the same ABI as Windows: one pointer for each type. This frees us from having to #include <pthread.h> and avoids the problem with pthread_rwlock_t not being defined under certain compiler defines. A few more changes are expected to the ABI -- they will be committed separately.
Created attachment 198054 [details] [review] Re-enable 'include' testcase The bug is fixed now.
Created attachment 198055 [details] [review] locks: drop _INIT macros All locks are now zero-initialised, so we can drop the G_*_INIT macros for them. Adjust various users around GLib accordingly and change the docs.
Review of attachment 198055 [details] [review]: Looks generally good. I would like to see the zero-initialization mentioned explicitly in the docs, somewhere. ::: glib/gthread.c @@ +331,3 @@ * * A GRecMutex should only be accessed with the + * <function>g_rec_mutex_</function> functions. I see that you ditched the static initializers, but the init() functions are still around, yes ? Should they not still be mentioned here ? And maybe it would be good to state explicitly that static locks/mutexes/whatever are usable as is, due to implicit zero-initialization by the compiler ?
Review of attachment 198053 [details] [review]: Looks good. ::: glib/gthread-posix.c @@ +84,3 @@ + gint status; + + mutex = malloc (sizeof (pthread_mutex_t)); This is system malloc because we can't use g_malloc() here ?
Review of attachment 198054 [details] [review]: no problem
Review of attachment 198055 [details] [review]: ::: glib/gthread-posix.c @@ +316,3 @@ * It is not necessary to initialize a recursive mutex that has + * been created with g_rec_mutex_new(). It is not necessary to + * initialise a recursive mutex that has been statically allocated. i mention that here, one for each type. wouldn't hurt to add it back to the docs for the structure type itself, of course.
Review of attachment 198053 [details] [review]: ::: glib/gthread-posix.c @@ +84,3 @@ + gint status; + + mutex = malloc (sizeof (pthread_mutex_t)); Yes -- g_malloc() uses locks, so locks can't use g_malloc().
fwiw, this malloc() business probably looks sort of silly, but in my defence: - we will replace GMutex and GCond with native implementations on Linux that use only the space already in the structure itself (no malloc) - Windows uses SRWLock and CONDITION_VARIABLE (no malloc) - FreeBSD and OpenBSD have pointer-sized pthread_mutex_t et al. that we can store directly into the structure (no malloc). - people used to use g_mutex_new() which involved a malloc anyway, and now they have g_mutex_init() to avoid that, so even in the worst case they are no worse off. - GRWLock and GRecMutex are vastly faster than the deprecated 'Static' ones
Attachment 198053 [details] pushed as e081ead - GThread posix: switch to Windows ABI Attachment 198054 [details] pushed as 3315aee - Re-enable 'include' testcase Attachment 198055 [details] pushed as 2a677d1 - locks: drop _INIT macros