Date: Fri, 24 Sep 2010 17:11:02 -0400 From: Jung-uk Kim <jkim@FreeBSD.org> To: John Baldwin <jhb@freebsd.org> Cc: Daniel Eischen <deischen@freebsd.org>, freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL Message-ID: <201009241711.16129.jkim@FreeBSD.org> In-Reply-To: <201009241137.56764.jkim@FreeBSD.org> References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009240926.12958.jhb@freebsd.org> <201009241137.56764.jkim@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_0PRnME9TC8tgtzT
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
On Friday 24 September 2010 11:37 am, Jung-uk Kim wrote:
> On Friday 24 September 2010 09:26 am, John Baldwin wrote:
> > On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote:
> > > On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote:
> > > > You shouldn't have to call pthread_mutex_init() on a mutex
> > > > initialized with PTHREAD_MUTEX_INITIALIZER. Our
> > > > implementation should auto initialize the mutex when it is
> > > > first used; if it doesn't, I think that is a bug.
> > >
> > > Ah, I see. I verified that libthr does it correctly. However,
> > > that's a hack and it is far from real static allocation
> > > although it should work pretty well in reality, IMHO. More
> > > over, it will have a side-effect, i.e., any destroyed mutex may
> > > be resurrected if it is used again. POSIX seems to say it
> > > should return EINVAL when it happens. :-(
> >
> > I think the fix there is that we should put a different value
> > ((void *)1 for example) into "destroyed" mutex objects than 0 so
> > that destroyed mutexes can be differentiated from statically
> > initialized mutexes. This would also allow us to properly return
> > EBUSY, etc.
>
> It would be nice if we had "uninitialized" as (void *)0 and "static
> initializer" as (void *)1. IMHO, it looks more natural, i.e.,
> "uninitialized" or "destroyed" one gets zero, and "dynamically
> initialized" or "statically initialized" one gets non-zero. Can we
> break the ABI for 9, maybe? ;-)
--------------------------------
WARNING!!! WARNING!!! WARNING!!!
--------------------------------
This patch is a proof of concept!
This patch massively breaks pthread ABI!
This patch may crash and burn your system!
I couldn't resist trying. ;-)
Jung-uk Kim
--Boundary-00=_0PRnME9TC8tgtzT
Content-Type: text/plain;
charset="iso-8859-1";
name="pthread.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="pthread.diff"
--- include/pthread.h 2009-03-14 16:10:14.000000000 -0400
+++ include/pthread.h 2010-09-24 16:34:45.000000000 -0400
@@ -97,10 +97,10 @@
/*
* Static initialization values.
*/
-#define PTHREAD_MUTEX_INITIALIZER NULL
-#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP NULL
-#define PTHREAD_COND_INITIALIZER NULL
-#define PTHREAD_RWLOCK_INITIALIZER NULL
+#define PTHREAD_MUTEX_INITIALIZER ((void *)1)
+#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP ((void *)1)
+#define PTHREAD_COND_INITIALIZER ((void *)1)
+#define PTHREAD_RWLOCK_INITIALIZER ((void *)1)
/*
* Default attribute arguments (draft 4, deprecated).
--- lib/libthr/thread/thr_cond.c 2010-09-01 12:13:31.000000000 -0400
+++ lib/libthr/thread/thr_cond.c 2010-09-24 15:36:56.000000000 -0400
@@ -94,7 +94,7 @@ init_static(struct pthread *thread, pthr
THR_LOCK_ACQUIRE(thread, &_cond_static_lock);
- if (*cond == NULL)
+ if (*cond == STATIC_INITIALIZER)
ret = cond_init(cond, NULL);
else
ret = 0;
@@ -121,6 +121,8 @@ _pthread_cond_destroy(pthread_cond_t *co
if (*cond == NULL)
rval = EINVAL;
+ else if (*cond == STATIC_INITIALIZER)
+ *cond = NULL;
else {
cv = *cond;
THR_UMUTEX_LOCK(curthread, &cv->c_lock);
@@ -184,7 +186,7 @@ cond_wait_common(pthread_cond_t *cond, p
* If the condition variable is statically initialized,
* perform the dynamic initialization:
*/
- if (__predict_false(*cond == NULL &&
+ if (__predict_false(*cond == STATIC_INITIALIZER &&
(ret = init_static(curthread, cond)) != 0))
return (ret);
@@ -273,7 +275,7 @@ cond_signal_common(pthread_cond_t *cond,
* If the condition variable is statically initialized, perform dynamic
* initialization.
*/
- if (__predict_false(*cond == NULL &&
+ if (__predict_false(*cond == STATIC_INITIALIZER &&
(ret = init_static(curthread, cond)) != 0))
return (ret);
--- lib/libthr/thread/thr_mutex.c 2010-09-01 12:13:32.000000000 -0400
+++ lib/libthr/thread/thr_mutex.c 2010-09-24 16:32:07.000000000 -0400
@@ -184,7 +184,7 @@ init_static(struct pthread *thread, pthr
THR_LOCK_ACQUIRE(thread, &_mutex_static_lock);
- if (*mutex == NULL)
+ if (*mutex == STATIC_INITIALIZER)
ret = mutex_init(mutex, NULL, calloc);
else
ret = 0;
@@ -263,6 +263,8 @@ _pthread_mutex_destroy(pthread_mutex_t *
if (__predict_false(*mutex == NULL))
ret = EINVAL;
+ else if (*mutex == STATIC_INITIALIZER)
+ *mutex = NULL;
else {
id = TID(curthread);
@@ -346,7 +348,7 @@ __pthread_mutex_trylock(pthread_mutex_t
* If the mutex is statically initialized, perform the dynamic
* initialization:
*/
- if (__predict_false(*mutex == NULL)) {
+ if (__predict_false(*mutex == STATIC_INITIALIZER)) {
ret = init_static(curthread, mutex);
if (__predict_false(ret))
return (ret);
@@ -454,7 +456,7 @@ __pthread_mutex_lock(pthread_mutex_t *mu
* If the mutex is statically initialized, perform the dynamic
* initialization:
*/
- if (__predict_false((m = *mutex) == NULL)) {
+ if (__predict_false((m = *mutex) == STATIC_INITIALIZER)) {
ret = init_static(curthread, mutex);
if (__predict_false(ret))
return (ret);
@@ -479,7 +481,7 @@ __pthread_mutex_timedlock(pthread_mutex_
* If the mutex is statically initialized, perform the dynamic
* initialization:
*/
- if (__predict_false((m = *mutex) == NULL)) {
+ if (__predict_false((m = *mutex) == STATIC_INITIALIZER)) {
ret = init_static(curthread, mutex);
if (__predict_false(ret))
return (ret);
@@ -614,6 +616,9 @@ mutex_unlock_common(pthread_mutex_t *mut
if (__predict_false((m = *mutex) == NULL))
return (EINVAL);
+ if (*mutex == STATIC_INITIALIZER)
+ return (EPERM);
+
/*
* Check if the running thread is not the owner of the mutex.
*/
@@ -749,7 +754,7 @@ __pthread_mutex_setspinloops_np(pthread_
struct pthread *curthread = _get_curthread();
int ret;
- if (__predict_false(*mutex == NULL)) {
+ if (__predict_false(*mutex == STATIC_INITIALIZER)) {
ret = init_static(curthread, mutex);
if (__predict_false(ret))
return (ret);
@@ -773,7 +778,7 @@ __pthread_mutex_setyieldloops_np(pthread
struct pthread *curthread = _get_curthread();
int ret;
- if (__predict_false(*mutex == NULL)) {
+ if (__predict_false(*mutex == STATIC_INITIALIZER)) {
ret = init_static(curthread, mutex);
if (__predict_false(ret))
return (ret);
@@ -788,7 +793,7 @@ _pthread_mutex_isowned_np(pthread_mutex_
struct pthread *curthread = _get_curthread();
int ret;
- if (__predict_false(*mutex == NULL)) {
+ if (__predict_false(*mutex == STATIC_INITIALIZER)) {
ret = init_static(curthread, mutex);
if (__predict_false(ret))
return (ret);
--- lib/libthr/thread/thr_private.h 2010-09-24 12:01:02.000000000 -0400
+++ lib/libthr/thread/thr_private.h 2010-09-24 14:43:35.000000000 -0400
@@ -125,6 +125,8 @@ TAILQ_HEAD(mutex_queue, pthread_mutex);
} \
} while (0)
+#define STATIC_INITIALIZER ((void *)1)
+
struct pthread_mutex {
/*
* Lock for accesses to this structure.
--- lib/libthr/thread/thr_rwlock.c 2009-07-06 05:31:04.000000000 -0400
+++ lib/libthr/thread/thr_rwlock.c 2010-09-24 16:45:37.000000000 -0400
@@ -64,10 +64,12 @@ rwlock_init(pthread_rwlock_t *rwlock, co
int
_pthread_rwlock_destroy (pthread_rwlock_t *rwlock)
{
- int ret;
+ int ret = 0;
if (rwlock == NULL)
ret = EINVAL;
+ else if (*rwlock == STATIC_INITIALIZER)
+ *rwlock = NULL;
else {
pthread_rwlock_t prwlock;
@@ -75,7 +77,6 @@ _pthread_rwlock_destroy (pthread_rwlock_
*rwlock = NULL;
free(prwlock);
- ret = 0;
}
return (ret);
}
@@ -87,7 +88,7 @@ init_static(struct pthread *thread, pthr
THR_LOCK_ACQUIRE(thread, &_rwlock_static_lock);
- if (*rwlock == NULL)
+ if (*rwlock == STATIC_INITIALIZER)
ret = rwlock_init(rwlock, NULL);
else
ret = 0;
@@ -119,7 +120,7 @@ rwlock_rdlock_common(pthread_rwlock_t *r
prwlock = *rwlock;
/* check for static initialization */
- if (__predict_false(prwlock == NULL)) {
+ if (__predict_false(prwlock == STATIC_INITIALIZER)) {
if ((ret = init_static(curthread, rwlock)) != 0)
return (ret);
@@ -212,7 +213,7 @@ _pthread_rwlock_tryrdlock (pthread_rwloc
prwlock = *rwlock;
/* check for static initialization */
- if (__predict_false(prwlock == NULL)) {
+ if (__predict_false(prwlock == STATIC_INITIALIZER)) {
if ((ret = init_static(curthread, rwlock)) != 0)
return (ret);
@@ -256,7 +257,7 @@ _pthread_rwlock_trywrlock (pthread_rwloc
prwlock = *rwlock;
/* check for static initialization */
- if (__predict_false(prwlock == NULL)) {
+ if (__predict_false(prwlock == STATIC_INITIALIZER)) {
if ((ret = init_static(curthread, rwlock)) != 0)
return (ret);
@@ -283,7 +284,7 @@ rwlock_wrlock_common (pthread_rwlock_t *
prwlock = *rwlock;
/* check for static initialization */
- if (__predict_false(prwlock == NULL)) {
+ if (__predict_false(prwlock == STATIC_INITIALIZER)) {
if ((ret = init_static(curthread, rwlock)) != 0)
return (ret);
@@ -364,6 +365,9 @@ _pthread_rwlock_unlock (pthread_rwlock_t
if (__predict_false(prwlock == NULL))
return (EINVAL);
+ if (prwlock == STATIC_INITIALIZER)
+ return (EPERM);
+
state = prwlock->lock.rw_state;
if (state & URWLOCK_WRITE_OWNER) {
if (__predict_false(prwlock->owner != curthread))
--Boundary-00=_0PRnME9TC8tgtzT--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009241711.16129.jkim>