diff options
author | wtc%netscape.com <devnull@localhost> | 2001-09-28 01:27:07 +0000 |
---|---|---|
committer | wtc%netscape.com <devnull@localhost> | 2001-09-28 01:27:07 +0000 |
commit | 3d02ec916350489e795bda21c1d996e2940a652f (patch) | |
tree | ae03bb44c4ab120186185cd3d5cadba32dac550b | |
parent | 9eee09f99d10e83bd38311d4da1ff04b2b46f6af (diff) | |
download | nspr-hg-3d02ec916350489e795bda21c1d996e2940a652f.tar.gz |
Bugzilla bug 84035: backed out the changes to the PRMonitor functions
because some of the Mozilla client code depends on the property that
PR_EnterMonitor and PR_ExitMonitor do not call malloc, calloc, and free.
Modified Files: _pth.h, primpl.h, ptsynch.c
-rw-r--r-- | pr/include/md/_pth.h | 46 | ||||
-rw-r--r-- | pr/include/private/primpl.h | 2 | ||||
-rw-r--r-- | pr/src/pthreads/ptsynch.c | 41 |
3 files changed, 69 insertions, 20 deletions
diff --git a/pr/include/md/_pth.h b/pr/include/md/_pth.h index 997f32f7..be65ab74 100644 --- a/pr/include/md/_pth.h +++ b/pr/include/md/_pth.h @@ -96,6 +96,52 @@ #define _PT_PTHREAD_COND_INIT(m, a) pthread_cond_init(&(m), &(a)) #endif +/* The pthreads standard does not specify an invalid value for the + * pthread_t handle. (0 is usually an invalid pthread identifier + * but there are exceptions, for example, DG/UX.) These macros + * define a way to set the handle to or compare the handle with an + * invalid identifier. These macros are not portable and may be + * more of a problem as we adapt to more pthreads implementations. + * They are only used in the PRMonitor functions. Do not use them + * in new code. + * + * Unfortunately some of our clients depend on certain properties + * of our PRMonitor implementation, preventing us from replacing + * it by a portable implementation. + * - High-performance servers like the fact that PR_EnterMonitor + * only calls PR_Lock and PR_ExitMonitor only calls PR_Unlock. + * (A portable implementation would use a PRLock and a PRCondVar + * to implement the recursive lock in a monitor and call both + * PR_Lock and PR_Unlock in PR_EnterMonitor and PR_ExitMonitor.) + * Unfortunately this forces us to read the monitor owner field + * without holding a lock. + * - One way to make it safe to read the monitor owner field + * without holding a lock is to make that field a PRThread* + * (one should be able to read a pointer with a single machine + * instruction). However, PR_GetCurrentThread calls calloc if + * it is called by a thread that was not created by NSPR. The + * malloc tracing tools in the Mozilla client use PRMonitor for + * locking in their malloc, calloc, and free functions. If + * PR_EnterMonitor calls any of these functions, infinite + * recursion ensues. + */ +#if defined(_PR_DCETHREADS) +#define _PT_PTHREAD_INVALIDATE_THR_HANDLE(t) \ + memset(&(t), 0, sizeof(pthread_t)) +#define _PT_PTHREAD_THR_HANDLE_IS_INVALID(t) \ + (!memcmp(&(t), &pt_zero_tid, sizeof(pthread_t))) +#define _PT_PTHREAD_COPY_THR_HANDLE(st, dt) (dt) = (st) +#elif defined(IRIX) || defined(OSF1) || defined(AIX) || defined(SOLARIS) \ + || defined(HPUX) || defined(LINUX) || defined(FREEBSD) \ + || defined(NETBSD) || defined(OPENBSD) || defined(BSDI) \ + || defined(VMS) || defined(NTO) || defined(RHAPSODY) +#define _PT_PTHREAD_INVALIDATE_THR_HANDLE(t) (t) = 0 +#define _PT_PTHREAD_THR_HANDLE_IS_INVALID(t) (t) == 0 +#define _PT_PTHREAD_COPY_THR_HANDLE(st, dt) (dt) = (st) +#else +#error "pthreads is not supported for this architecture" +#endif + #if defined(_PR_DCETHREADS) #define _PT_PTHREAD_ATTR_INIT pthread_attr_create #define _PT_PTHREAD_ATTR_DESTROY pthread_attr_delete diff --git a/pr/include/private/primpl.h b/pr/include/private/primpl.h index 7fa9d278..27567454 100644 --- a/pr/include/private/primpl.h +++ b/pr/include/private/primpl.h @@ -1456,7 +1456,7 @@ struct PRMonitor { const char* name; /* monitor name for debugging */ #if defined(_PR_PTHREADS) PRLock lock; /* the lock structure */ - PRThread *owner; /* the owner of the lock or NULL */ + pthread_t owner; /* the owner of the lock or invalid */ PRCondVar *cvar; /* condition variable queue */ #else /* defined(_PR_PTHREADS) */ PRCondVar *cvar; /* associated lock and condition variable queue */ diff --git a/pr/src/pthreads/ptsynch.c b/pr/src/pthreads/ptsynch.c index ecf69a1a..4400e8df 100644 --- a/pr/src/pthreads/ptsynch.c +++ b/pr/src/pthreads/ptsynch.c @@ -444,6 +444,8 @@ PR_IMPLEMENT(PRMonitor*) PR_NewMonitor(void) rv = _PT_PTHREAD_MUTEX_INIT(mon->lock.mutex, _pt_mattr); PR_ASSERT(0 == rv); + _PT_PTHREAD_INVALIDATE_THR_HANDLE(mon->owner); + mon->cvar = cvar; rv = _PT_PTHREAD_COND_INIT(mon->cvar->cv, _pt_cvar_attr); PR_ASSERT(0 == rv); @@ -484,42 +486,43 @@ PR_IMPLEMENT(void) PR_DestroyMonitor(PRMonitor *mon) */ PR_IMPLEMENT(PRInt32) PR_GetMonitorEntryCount(PRMonitor *mon) { - PRThread *self = PR_GetCurrentThread(); - if (mon->owner == self) + pthread_t self = pthread_self(); + if (pthread_equal(mon->owner, self)) return mon->entryCount; return 0; } PR_IMPLEMENT(void) PR_EnterMonitor(PRMonitor *mon) { - PRThread *self = PR_GetCurrentThread(); + pthread_t self = pthread_self(); PR_ASSERT(mon != NULL); /* - * This is safe only if mon->owner (a PRThread*) can be - * read in one instruction. + * This is safe only if mon->owner (a pthread_t) can be + * read in one instruction. Perhaps mon->owner should be + * a "PRThread *"? */ - if (mon->owner != self) + if (!pthread_equal(mon->owner, self)) { PR_Lock(&mon->lock); /* and now I have the lock */ PR_ASSERT(0 == mon->entryCount); - PR_ASSERT(NULL == mon->owner); - mon->owner = self; + PR_ASSERT(_PT_PTHREAD_THR_HANDLE_IS_INVALID(mon->owner)); + _PT_PTHREAD_COPY_THR_HANDLE(self, mon->owner); } mon->entryCount += 1; } /* PR_EnterMonitor */ PR_IMPLEMENT(PRStatus) PR_ExitMonitor(PRMonitor *mon) { - PRThread *self = PR_GetCurrentThread(); + pthread_t self = pthread_self(); PR_ASSERT(mon != NULL); /* The lock better be that - locked */ PR_ASSERT(_PT_PTHREAD_MUTEX_IS_LOCKED(mon->lock.mutex)); /* we'd better be the owner */ - PR_ASSERT(mon->owner == self); - if (mon->owner != self) + PR_ASSERT(pthread_equal(mon->owner, self)); + if (!pthread_equal(mon->owner, self)) return PR_FAILURE; /* if it's locked and we have it, then the entries should be > 0 */ @@ -528,7 +531,7 @@ PR_IMPLEMENT(PRStatus) PR_ExitMonitor(PRMonitor *mon) if (mon->entryCount == 0) { /* and if it transitioned to zero - unlock */ - mon->owner = NULL; /* make the owner unknown */ + _PT_PTHREAD_INVALIDATE_THR_HANDLE(mon->owner); /* make the owner unknown */ PR_Unlock(&mon->lock); } return PR_SUCCESS; @@ -538,7 +541,7 @@ PR_IMPLEMENT(PRStatus) PR_Wait(PRMonitor *mon, PRIntervalTime timeout) { PRStatus rv; PRInt16 saved_entries; - PRThread *saved_owner; + pthread_t saved_owner; PR_ASSERT(mon != NULL); /* we'd better be locked */ @@ -546,19 +549,19 @@ PR_IMPLEMENT(PRStatus) PR_Wait(PRMonitor *mon, PRIntervalTime timeout) /* and the entries better be positive */ PR_ASSERT(mon->entryCount > 0); /* and it better be by us */ - PR_ASSERT(mon->owner == PR_GetCurrentThread()); + PR_ASSERT(pthread_equal(mon->owner, pthread_self())); /* tuck these away 'till later */ saved_entries = mon->entryCount; mon->entryCount = 0; - saved_owner = mon->owner; - mon->owner = NULL; + _PT_PTHREAD_COPY_THR_HANDLE(mon->owner, saved_owner); + _PT_PTHREAD_INVALIDATE_THR_HANDLE(mon->owner); rv = PR_WaitCondVar(mon->cvar, timeout); /* reinstate the intresting information */ mon->entryCount = saved_entries; - mon->owner = saved_owner; + _PT_PTHREAD_COPY_THR_HANDLE(saved_owner, mon->owner); return rv; } /* PR_Wait */ @@ -571,7 +574,7 @@ PR_IMPLEMENT(PRStatus) PR_Notify(PRMonitor *mon) /* and the entries better be positive */ PR_ASSERT(mon->entryCount > 0); /* and it better be by us */ - PR_ASSERT(mon->owner == PR_GetCurrentThread()); + PR_ASSERT(pthread_equal(mon->owner, pthread_self())); pt_PostNotifyToCvar(mon->cvar, PR_FALSE); @@ -586,7 +589,7 @@ PR_IMPLEMENT(PRStatus) PR_NotifyAll(PRMonitor *mon) /* and the entries better be positive */ PR_ASSERT(mon->entryCount > 0); /* and it better be by us */ - PR_ASSERT(mon->owner == PR_GetCurrentThread()); + PR_ASSERT(pthread_equal(mon->owner, pthread_self())); pt_PostNotifyToCvar(mon->cvar, PR_TRUE); |