[
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-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org
