You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-dev@xerces.apache.org by "Boris Kolpackov (JIRA)" <xe...@xml.apache.org> on 2009/11/03 09:29:59 UTC
[jira] Closed: (XERCESC-1762) Tru64 PlatformUtils RecursiveMutex
implementation is not thread-safe
[ https://issues.apache.org/jira/browse/XERCESC-1762?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Boris Kolpackov closed XERCESC-1762.
------------------------------------
Resolution: Fixed
This code is no longer in 3-series.
> Tru64 PlatformUtils RecursiveMutex implementation is not thread-safe
> --------------------------------------------------------------------
>
> Key: XERCESC-1762
> URL: https://issues.apache.org/jira/browse/XERCESC-1762
> Project: Xerces-C++
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 2.7.0, 2.8.0
> Environment: Tru64 5.1
> Reporter: Vladimir Lazarenko
> Priority: Critical
>
> The way, how RecursiveMutex is implemented is not thread safe. We've come up with a different implementation and herewith would like to share it with you, with hope that it will make its way into the mainstream.
> In essence the issue is that you can not check if you're the owner of the thread until you lock. If you don't lock, the tid can be overwritten, and the check will be bogus. Further on, gAtomicOpMutex could use PTHREAD_MUTEX_INITIALIZER instead of custom initialization.
> -bash-2.05b$ svn diff -r21597 Tru64PlatformUtils.cpp
> Index: Tru64PlatformUtils.cpp
> ===================================================================
> --- Tru64PlatformUtils.cpp (revision 21597)
> +++ Tru64PlatformUtils.cpp (working copy)
> @@ -42,7 +42,10 @@
> #include <xercesc/util/XMLUniDefs.hpp>
> #include <xercesc/util/PanicHandler.hpp>
> #include <xercesc/util/OutOfMemoryException.hpp>
> +#include <xercesc/util/XMemory.hpp>
> +#include <assert.h>
> +
> //
> // These control which transcoding service is used by the Tru64 version.
> // They allow this to be controlled from the build process by just defining
> @@ -397,9 +400,10 @@
> // XMLPlatformUtils: Platform init method
> // ---------------------------------------------------------------------------
> -typedef XMLHolder<pthread_mutex_t> MutexHolderType;
> +//typedef XMLHolder<pthread_mutex_t> MutexHolderType;
> -static MutexHolderType* gAtomicOpMutex = 0;
> +static pthread_mutex_t gAtomicOpMutex = PTHREAD_MUTEX_INITIALIZER;
> +//static MutexHolderType* gAtomicOpMutex = 0;
> void XMLPlatformUtils::platformInit()
> {
> @@ -411,14 +415,15 @@
> // circular dependency between compareAndExchange() and
> // mutex creation that must be broken.
> +#if 0
> gAtomicOpMutex = new (fgMemoryManager) MutexHolderType;
> -
> if (pthread_mutex_init(&gAtomicOpMutex->fInstance, NULL)) {
> delete gAtomicOpMutex;
> gAtomicOpMutex = 0;
> panic( PanicHandler::Panic_SystemInit );
> }
> +#endif
> }
> @@ -427,61 +432,81 @@
> // -----------------------------------------------------------------------
> -class RecursiveMutex
> +class RecursiveMutex : public XMemory
> {
> -public:
> - pthread_mutex_t mutex;
> - int recursionCount;
> - pthread_t tid;
> - MemoryManager* const fMemoryManager;
> +public :
> - RecursiveMutex(MemoryManager* manager) :
> - mutex(),
> - recursionCount(0),
> - tid(0),
> - fMemoryManager(manager)
> + RecursiveMutex()
> + : mut(),
> + cond(),
> + count(0),
> + owner()
> {
> - if (pthread_mutex_init(&mutex, NULL))
> - XMLPlatformUtils::panic(PanicHandler::Panic_MutexErr);
> + int r = pthread_mutex_init(&mut, 0);
> + if (r != 0) {
> + XMLPlatformUtils::panic(PanicHandler::Panic_MutexErr);
> + }
> + r = pthread_cond_init(&cond, 0);
> + if (r != 0) {
> + pthread_mutex_destroy(&mut);
> + XMLPlatformUtils::panic(PanicHandler::Panic_MutexErr);
> + }
> }
> - ~RecursiveMutex()
> + void lock()
> {
> - if (pthread_mutex_destroy(&mutex))
> - ThrowXMLwithMemMgr(XMLPlatformUtilsException, XMLExcepts::Mutex_CouldNotDestroy, fMemoryManager);
> + pthread_t self = pthread_self();
> +
> + int r = pthread_mutex_lock(&mut);
> + assert(r == 0);
> +
> + while (count != 0 && ! pthread_equal(self, owner)) {
> + r = pthread_cond_wait(&cond, &mut);
> + assert(r == 0);
> + }
> +
> + if (count++ == 0) {
> + owner = self;
> + }
> +
> + r = pthread_mutex_unlock(&mut);
> + assert(r == 0);
> }
> - void lock()
> + void unlock()
> {
> - if (pthread_equal(tid, pthread_self()))
> - {
> - recursionCount++;
> - return;
> + int r = pthread_mutex_lock(&mut);
> + assert(r == 0);
> +
> + assert(pthread_equal(pthread_self(), owner));
> +
> + if (--count == 0) {
> + pthread_cond_signal(&cond);
> }
> - if (pthread_mutex_lock(&mutex) != 0)
> - XMLPlatformUtils::panic(PanicHandler::Panic_MutexErr);
> - tid = pthread_self();
> - recursionCount = 1;
> +
> + r = pthread_mutex_unlock(&mut);
> + assert(r == 0);
> }
> -
> - void unlock()
> + ~RecursiveMutex()
> {
> - if (--recursionCount > 0)
> - return;
> + assert(count == 0);
> + pthread_cond_destroy(&cond);
> + pthread_mutex_destroy(&mut);
> + }
> - if (pthread_mutex_unlock(&mutex) != 0)
> - XMLPlatformUtils::panic(PanicHandler::Panic_MutexErr);
> - tid = 0;
> - }
> +private :
> + pthread_mutex_t mut;
> + pthread_cond_t cond;
> + unsigned int count;
> + pthread_t owner; // valid if count != 0
> };
> void* XMLPlatformUtils::makeMutex(MemoryManager* manager)
> {
> - return new (manager) RecursiveMutex(manager);
> + return new (manager) RecursiveMutex;
> }
> -
> void XMLPlatformUtils::closeMutex(void* const mtxHandle)
> {
> if (mtxHandle == NULL)
> @@ -522,14 +547,14 @@
> // the below calls are temporarily made till the above functions are part of user library
> // Currently its supported only in the kernel mode
> - if (pthread_mutex_lock( &gAtomicOpMutex->fInstance))
> + if (pthread_mutex_lock( &gAtomicOpMutex))
> panic(PanicHandler::Panic_SynchronizationErr);
> void *retVal = *toFill;
> if (*toFill == toCompare)
> *toFill = (void *)newValue;
> - if (pthread_mutex_unlock( &gAtomicOpMutex->fInstance))
> + if (pthread_mutex_unlock( &gAtomicOpMutex))
> panic(PanicHandler::Panic_SynchronizationErr);
> return retVal;
> @@ -539,12 +564,12 @@
> {
> //return (int)atomic_add_32_nv( (uint32_t*)&location, 1);
> - if (pthread_mutex_lock( &gAtomicOpMutex->fInstance))
> + if (pthread_mutex_lock( &gAtomicOpMutex))
> panic(PanicHandler::Panic_SynchronizationErr);
> int tmp = ++location;
> - if (pthread_mutex_unlock( &gAtomicOpMutex->fInstance))
> + if (pthread_mutex_unlock( &gAtomicOpMutex))
> panic(PanicHandler::Panic_SynchronizationErr);
> return tmp;
> @@ -553,12 +578,12 @@
> {
> //return (int)atomic_add_32_nv( (uint32_t*)&location, -1);
> - if (pthread_mutex_lock( &gAtomicOpMutex->fInstance))
> + if (pthread_mutex_lock( &gAtomicOpMutex))
> panic(PanicHandler::Panic_SynchronizationErr);
> int tmp = --location;
> - if (pthread_mutex_unlock( &gAtomicOpMutex->fInstance))
> + if (pthread_mutex_unlock( &gAtomicOpMutex))
> panic(PanicHandler::Panic_SynchronizationErr);
> return tmp;
> @@ -619,11 +644,6 @@
> void XMLPlatformUtils::platformTerm()
> {
> -#if !defined (APP_NO_THREADS)
> - pthread_mutex_destroy(&gAtomicOpMutex->fInstance);
> - delete gAtomicOpMutex;
> - gAtomicOpMutex = 0;
> -#endif
> }
> #include <xercesc/util/LogicalPath.c>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org