From 5177298dad0efafbf17c695ba8afecb944d474ab Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Tue, 4 Nov 2014 20:06:05 +0000 Subject: [PATCH] Change isRunning variable to an Atomic to address possible race condition asscoiated with reading and writing to the variable from different threads. git-svn-id: http://svn.openscenegraph.org/osg/OpenSceneGraph/trunk@14464 16af8721-9629-0410-8352-f15c8da7e697 --- src/OpenThreads/pthreads/PThread.cpp | 20 +++++++++---------- src/OpenThreads/pthreads/PThreadPrivateData.h | 20 +++++++++++-------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/OpenThreads/pthreads/PThread.cpp b/src/OpenThreads/pthreads/PThread.cpp index f01bc1675..66e97c96a 100644 --- a/src/OpenThreads/pthreads/PThread.cpp +++ b/src/OpenThreads/pthreads/PThread.cpp @@ -83,7 +83,7 @@ struct ThreadCleanupStruct { OpenThreads::Thread *thread; - volatile bool *runflag; + Atomic *runflag; }; @@ -97,7 +97,7 @@ void thread_cleanup_handler(void *arg) ThreadCleanupStruct *tcs = static_cast(arg); tcs->thread->cancelCleanup(); - *(tcs->runflag) = false; + (*(tcs->runflag)).exchange(0); } @@ -176,7 +176,7 @@ private: ThreadCleanupStruct tcs; tcs.thread = thread; - tcs.runflag = &pd->isRunning; + tcs.runflag = &pd->_isRunning; // Set local storage so that Thread::CurrentThread() can return the right thing int status = pthread_setspecific(PThreadPrivateData::s_tls_key, thread); @@ -196,14 +196,14 @@ private: #endif // ] ALLOW_PRIORITY_SCHEDULING - pd->isRunning = true; + pd->setRunning(true); // release the thread that created this thread. pd->threadStartedBlock.release(); thread->run(); - pd->isRunning = false; + pd->setRunning(false); pthread_cleanup_pop(0); @@ -426,7 +426,7 @@ Thread::Thread() pd->stackSize = 0; pd->stackSizeLocked = false; pd->idSet = false; - pd->isRunning = false; + pd->setRunning(false); pd->isCanceled = false; pd->uniqueId = pd->nextId; pd->nextId++; @@ -448,7 +448,7 @@ Thread::~Thread() { PThreadPrivateData *pd = static_cast(_prvData); - if(pd->isRunning) + if(pd->isRunning()) { std::cout<<"Error: Thread "<isRunning && Thread::CurrentThread()==this) + if (pd->isRunning() && Thread::CurrentThread()==this) { cpu_set_t cpumask; CPU_ZERO( &cpumask ); @@ -624,7 +624,7 @@ int Thread::setProcessorAffinity(unsigned int cpunum) bool Thread::isRunning() { PThreadPrivateData *pd = static_cast (_prvData); - return pd->isRunning; + return pd->isRunning(); } @@ -783,7 +783,7 @@ int Thread::cancel() { #if defined(HAVE_PTHREAD_CANCEL) PThreadPrivateData *pd = static_cast (_prvData); - if (pd->isRunning) + if (pd->isRunning()) { pd->isCanceled = true; int status = pthread_cancel(pd->tid); diff --git a/src/OpenThreads/pthreads/PThreadPrivateData.h b/src/OpenThreads/pthreads/PThreadPrivateData.h index a8297fd2b..0864affcb 100644 --- a/src/OpenThreads/pthreads/PThreadPrivateData.h +++ b/src/OpenThreads/pthreads/PThreadPrivateData.h @@ -1,13 +1,13 @@ /* -*-c++-*- OpenThreads library, Copyright (C) 2002 - 2007 The Open Thread Group * - * This library is open source and may be redistributed and/or modified under - * the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or + * This library is open source and may be redistributed and/or modified under + * the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or * (at your option) any later version. The full license is in LICENSE file * included with this distribution, and on the openscenegraph.org website. - * + * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * OpenSceneGraph Public License for more details. */ @@ -22,6 +22,7 @@ #include #include #include +#include namespace OpenThreads { @@ -33,7 +34,7 @@ class PThreadPrivateData { friend class Thread; //------------------------------------------------------------------------- - // We're friendly to PThreadPrivateActions, so it can get at some + // We're friendly to PThreadPrivateActions, so it can get at some // variables. // friend class ThreadPrivateActions; @@ -48,7 +49,10 @@ private: volatile bool stackSizeLocked; - volatile bool isRunning; + void setRunning(bool flag) { _isRunning.exchange(flag); } + bool isRunning() const { return _isRunning!=0; } + + Atomic _isRunning; Block threadStartedBlock; @@ -57,7 +61,7 @@ private: volatile bool idSet; volatile Thread::ThreadPriority threadPriority; - + volatile Thread::ThreadPolicy threadPolicy; pthread_t tid; @@ -65,7 +69,7 @@ private: volatile int uniqueId; volatile int cpunum; - + static int nextId;