Browse Source

Fixed a data race in the Thread destructor

tags/2021-05-28
Tom Poole 8 years ago
parent
commit
c1a3aa38f8
4 changed files with 21 additions and 17 deletions
  1. +4
    -4
      modules/juce_core/native/juce_posix_SharedCode.h
  2. +3
    -3
      modules/juce_core/native/juce_win32_Threads.cpp
  3. +11
    -7
      modules/juce_core/threads/juce_Thread.cpp
  4. +3
    -3
      modules/juce_core/threads/juce_Thread.h

+ 4
- 4
modules/juce_core/native/juce_posix_SharedCode.h View File

@@ -944,7 +944,7 @@ void Thread::launchThread()
{ {
#if JUCE_ANDROID_REALTIME_THREAD_AVAILABLE #if JUCE_ANDROID_REALTIME_THREAD_AVAILABLE
threadHandle = (void*) juce_createRealtimeAudioThread (threadEntryProc, this); threadHandle = (void*) juce_createRealtimeAudioThread (threadEntryProc, this);
threadId = (ThreadID) threadHandle;
threadId = (ThreadID) threadHandle.get();
return; return;
#else #else
@@ -969,7 +969,7 @@ void Thread::launchThread()
{ {
pthread_detach (handle); pthread_detach (handle);
threadHandle = (void*) handle; threadHandle = (void*) handle;
threadId = (ThreadID) threadHandle;
threadId = (ThreadID) threadHandle.get();
} }
if (attrPtr != nullptr) if (attrPtr != nullptr)
@@ -984,12 +984,12 @@ void Thread::closeThreadHandle()
void Thread::killThread() void Thread::killThread()
{ {
if (threadHandle != 0)
if (threadHandle.get() != 0)
{ {
#if JUCE_ANDROID #if JUCE_ANDROID
jassertfalse; // pthread_cancel not available! jassertfalse; // pthread_cancel not available!
#else #else
pthread_cancel ((pthread_t) threadHandle);
pthread_cancel ((pthread_t) threadHandle.get());
#endif #endif
} }
} }


+ 3
- 3
modules/juce_core/native/juce_win32_Threads.cpp View File

@@ -87,19 +87,19 @@ void Thread::launchThread()
void Thread::closeThreadHandle() void Thread::closeThreadHandle()
{ {
CloseHandle ((HANDLE) threadHandle);
CloseHandle ((HANDLE) threadHandle.get());
threadId = 0; threadId = 0;
threadHandle = 0; threadHandle = 0;
} }
void Thread::killThread() void Thread::killThread()
{ {
if (threadHandle != 0)
if (threadHandle.get() != 0)
{ {
#if JUCE_DEBUG #if JUCE_DEBUG
OutputDebugStringA ("** Warning - Forced thread termination **\n"); OutputDebugStringA ("** Warning - Forced thread termination **\n");
#endif #endif
TerminateThread (threadHandle, 0);
TerminateThread (threadHandle.get(), 0);
} }
} }


+ 11
- 7
modules/juce_core/threads/juce_Thread.cpp View File

@@ -86,7 +86,7 @@ void Thread::threadEntryPoint()
if (startSuspensionEvent.wait (10000)) if (startSuspensionEvent.wait (10000))
{ {
jassert (getCurrentThreadId() == threadId);
jassert (getCurrentThreadId() == threadId.get());
if (affinityMask != 0) if (affinityMask != 0)
setCurrentThreadAffinityMask (affinityMask); setCurrentThreadAffinityMask (affinityMask);
@@ -102,9 +102,13 @@ void Thread::threadEntryPoint()
} }
currentThreadHolder->value.releaseCurrentThreadStorage(); currentThreadHolder->value.releaseCurrentThreadStorage();
// Once closeThreadHandle is called this class may be deleted by a different
// thread, so we need to store deleteOnThreadEnd in a local variable.
auto shouldDeleteThis = deleteOnThreadEnd;
closeThreadHandle(); closeThreadHandle();
if (deleteOnThreadEnd)
if (shouldDeleteThis)
delete this; delete this;
} }
@@ -121,10 +125,10 @@ void Thread::startThread()
shouldExit = false; shouldExit = false;
if (threadHandle == nullptr)
if (threadHandle.get() == nullptr)
{ {
launchThread(); launchThread();
setThreadPriority (threadHandle, threadPriority);
setThreadPriority (threadHandle.get(), threadPriority);
startSuspensionEvent.signal(); startSuspensionEvent.signal();
} }
} }
@@ -133,7 +137,7 @@ void Thread::startThread (int priority)
{ {
const ScopedLock sl (startStopLock); const ScopedLock sl (startStopLock);
if (threadHandle == nullptr)
if (threadHandle.get() == nullptr)
{ {
auto isRealtime = (priority == realtimeAudioPriority); auto isRealtime = (priority == realtimeAudioPriority);
@@ -155,7 +159,7 @@ void Thread::startThread (int priority)
bool Thread::isThreadRunning() const bool Thread::isThreadRunning() const
{ {
return threadHandle != nullptr;
return threadHandle.get() != nullptr;
} }
Thread* JUCE_CALLTYPE Thread::getCurrentThread() Thread* JUCE_CALLTYPE Thread::getCurrentThread()
@@ -263,7 +267,7 @@ bool Thread::setPriority (int newPriority)
isAndroidRealtimeThread = isRealtime; isAndroidRealtimeThread = isRealtime;
#endif #endif
if ((! isThreadRunning()) || setThreadPriority (threadHandle, newPriority))
if ((! isThreadRunning()) || setThreadPriority (threadHandle.get(), newPriority))
{ {
threadPriority = newPriority; threadPriority = newPriority;
return true; return true;


+ 3
- 3
modules/juce_core/threads/juce_Thread.h View File

@@ -309,7 +309,7 @@ public:
thread's not actually running. thread's not actually running.
@see getCurrentThreadId @see getCurrentThreadId
*/ */
ThreadID getThreadId() const noexcept { return threadId; }
ThreadID getThreadId() const noexcept { return threadId.get(); }
/** Returns the name of the thread. /** Returns the name of the thread.
This is the name that gets set in the constructor. This is the name that gets set in the constructor.
@@ -325,8 +325,8 @@ public:
private: private:
//============================================================================== //==============================================================================
const String threadName; const String threadName;
void* volatile threadHandle = nullptr;
ThreadID threadId = {};
Atomic<void*> threadHandle { nullptr };
Atomic<ThreadID> threadId = {};
CriticalSection startStopLock; CriticalSection startStopLock;
WaitableEvent startSuspensionEvent, defaultEvent; WaitableEvent startSuspensionEvent, defaultEvent;
int threadPriority = 5; int threadPriority = 5;


Loading…
Cancel
Save