Rewrite ThreadPosix.
This is the same change as already made for Windows:
https://webrtc-codereview.appspot.com/37069004/
* Remove "dead" and "alive" variables.
* Remove critical section
* Remove implementation of SetNotAlive()
* Always set thread name
* Add thread checks for correct usage.
* Changed AudioDeviceMac to create/start/stop/delete thread objects for playout and recording, inside the respective start and stop method. The reason for this is because the AudioDeviceMac instance is currently being created on one thread and the above Start/Stop methods are being called on a different thread. So, my change makes creation, start/stop, deletion of the thread objects always happen on the same thread.
I'm making CurrentThreadId() in rtc_base_approved more visible so that it can be used from there instead of inside webrtc. Down the line we will have more thread concepts in rtc_base_approved, so I put a TODO for myself to move this functionality to there once we do.
R=henrika@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/40599004
Cr-Commit-Position: refs/heads/master@{#8235}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8235 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/base/thread_checker_impl.cc b/webrtc/base/thread_checker_impl.cc
index 48478e7..0b419d9 100644
--- a/webrtc/base/thread_checker_impl.cc
+++ b/webrtc/base/thread_checker_impl.cc
@@ -19,14 +19,14 @@
#endif
namespace rtc {
-namespace {
+
PlatformThreadId CurrentThreadId() {
#if defined(WEBRTC_WIN)
return GetCurrentThreadId();
#elif defined(WEBRTC_POSIX)
// Pthreads doesn't have the concept of a thread ID, so we have to reach down
// into the kernel.
-#if defined(WEBRTC_MAC)
+#if defined(WEBRTC_MAC) || defined(WEBRTC_IOS)
return pthread_mach_thread_np(pthread_self());
#elif defined(WEBRTC_LINUX)
return syscall(__NR_gettid);
@@ -38,7 +38,6 @@
#endif
#endif // defined(WEBRTC_POSIX)
}
-} // namespace
ThreadCheckerImpl::ThreadCheckerImpl() : valid_thread_(CurrentThreadId()) {
}
diff --git a/webrtc/base/thread_checker_impl.h b/webrtc/base/thread_checker_impl.h
index 5f851c4..67adc6e 100644
--- a/webrtc/base/thread_checker_impl.h
+++ b/webrtc/base/thread_checker_impl.h
@@ -29,6 +29,9 @@
typedef pid_t PlatformThreadId;
#endif
+// TODO(tommi): This+PlatformThreadId belongs in a common thread related header.
+PlatformThreadId CurrentThreadId();
+
// Real implementation of ThreadChecker, for use in debug mode, or
// for temporary use in release mode (e.g. to CHECK on a threading issue
// seen only in the wild).
diff --git a/webrtc/modules/audio_device/mac/audio_device_mac.cc b/webrtc/modules/audio_device/mac/audio_device_mac.cc
index 8a89793..3f39cdf 100644
--- a/webrtc/modules/audio_device/mac/audio_device_mac.cc
+++ b/webrtc/modules/audio_device/mac/audio_device_mac.cc
@@ -8,17 +8,16 @@
* be found in the AUTHORS file in the root of the source tree.
*/
+#include "webrtc/base/checks.h"
#include "webrtc/modules/audio_device/audio_device_config.h"
#include "webrtc/modules/audio_device/audio_device_utility.h"
#include "webrtc/modules/audio_device/mac/audio_device_mac.h"
-
#include "webrtc/modules/audio_device/mac/portaudio/pa_ringbuffer.h"
#include "webrtc/system_wrappers/interface/event_wrapper.h"
#include "webrtc/system_wrappers/interface/thread_wrapper.h"
#include "webrtc/system_wrappers/interface/trace.h"
#include <ApplicationServices/ApplicationServices.h>
-#include <assert.h>
#include <libkern/OSAtomic.h> // OSAtomicCompareAndSwap()
#include <mach/mach.h> // mach_task_self()
#include <sys/sysctl.h> // sysctlbyname()
@@ -56,8 +55,6 @@
} \
} while(0)
-#define ARRAY_SIZE(a) (sizeof(a)/sizeof((a)[0]))
-
enum
{
MaxNumberDevices = 64
@@ -94,8 +91,8 @@
const int32_t id, const char *msg,
const char *err)
{
- assert(msg != NULL);
- assert(err != NULL);
+ DCHECK(msg != NULL);
+ DCHECK(err != NULL);
#ifdef WEBRTC_ARCH_BIG_ENDIAN
WEBRTC_TRACE(level, module, id, "%s: %.4s", msg, err);
@@ -111,10 +108,8 @@
_critSect(*CriticalSectionWrapper::CreateCriticalSection()),
_stopEventRec(*EventWrapper::Create()),
_stopEvent(*EventWrapper::Create()),
- _captureWorkerThread(NULL),
- _renderWorkerThread(NULL),
- _captureWorkerThreadId(0),
- _renderWorkerThreadId(0),
+ capture_worker_thread_id_(0),
+ render_worker_thread_id_(0),
_id(id),
_mixerManager(id),
_inputDeviceIndex(0),
@@ -161,8 +156,8 @@
WEBRTC_TRACE(kTraceMemory, kTraceAudioDevice, id,
"%s created", __FUNCTION__);
- assert(&_stopEvent != NULL);
- assert(&_stopEventRec != NULL);
+ DCHECK(&_stopEvent != NULL);
+ DCHECK(&_stopEventRec != NULL);
memset(_renderConvertData, 0, sizeof(_renderConvertData));
memset(&_outStreamFormat, 0, sizeof(AudioStreamBasicDescription));
@@ -182,17 +177,8 @@
Terminate();
}
- if (_captureWorkerThread)
- {
- delete _captureWorkerThread;
- _captureWorkerThread = NULL;
- }
-
- if (_renderWorkerThread)
- {
- delete _renderWorkerThread;
- _renderWorkerThread = NULL;
- }
+ DCHECK(!capture_worker_thread_.get());
+ DCHECK(!render_worker_thread_.get());
if (_paRenderBuffer)
{
@@ -331,32 +317,6 @@
}
}
- if (_renderWorkerThread == NULL)
- {
- _renderWorkerThread
- = ThreadWrapper::CreateThread(RunRender, this, kRealtimePriority,
- "RenderWorkerThread");
- if (_renderWorkerThread == NULL)
- {
- WEBRTC_TRACE(kTraceCritical, kTraceAudioDevice,
- _id, " Render CreateThread() error");
- return -1;
- }
- }
-
- if (_captureWorkerThread == NULL)
- {
- _captureWorkerThread
- = ThreadWrapper::CreateThread(RunCapture, this, kRealtimePriority,
- "CaptureWorkerThread");
- if (_captureWorkerThread == NULL)
- {
- WEBRTC_TRACE(kTraceCritical, kTraceAudioDevice,
- _id, " Capture CreateThread() error");
- return -1;
- }
- }
-
kern_return_t kernErr = KERN_SUCCESS;
kernErr = semaphore_create(mach_task_self(), &_renderSemaphore,
SYNC_POLICY_FIFO, 0);
@@ -469,13 +429,13 @@
retVal = -1;
}
- _critSect.Leave();
-
_isShutDown = true;
_initialized = false;
_outputDeviceIsSpecified = false;
_inputDeviceIsSpecified = false;
+ _critSect.Leave();
+
return retVal;
}
@@ -1104,6 +1064,7 @@
int32_t AudioDeviceMac::SetPlayoutDevice(uint16_t index)
{
+ CriticalSectionScoped lock(&_critSect);
if (_playIsInitialized)
{
@@ -1287,7 +1248,6 @@
int32_t AudioDeviceMac::InitPlayout()
{
-
CriticalSectionScoped lock(&_critSect);
if (_playing)
@@ -1794,15 +1754,14 @@
return -1;
}
+ DCHECK(!capture_worker_thread_.get());
+ capture_worker_thread_.reset(
+ ThreadWrapper::CreateThread(RunCapture, this, kRealtimePriority,
+ "CaptureWorkerThread"));
+ DCHECK(capture_worker_thread_.get());
+ capture_worker_thread_->Start(capture_worker_thread_id_);
+
OSStatus err = noErr;
-
- unsigned int threadID(0);
- if (_captureWorkerThread != NULL)
- {
- _captureWorkerThread->Start(threadID);
- }
- _captureWorkerThreadId = threadID;
-
if (_twoDevices)
{
WEBRTC_CA_RETURN_ON_ERR(AudioDeviceStart(_inputDeviceID, _inDeviceIOProcID));
@@ -1893,17 +1852,13 @@
// Setting this signal will allow the worker thread to be stopped.
AtomicSet32(&_captureDeviceIsAlive, 0);
- _critSect.Leave();
- if (_captureWorkerThread != NULL)
- {
- if (!_captureWorkerThread->Stop())
- {
- WEBRTC_TRACE(kTraceError, kTraceAudioDevice, _id,
- " Timed out waiting for the render worker thread to "
- "stop.");
- }
+
+ if (capture_worker_thread_.get()) {
+ _critSect.Leave();
+ capture_worker_thread_->Stop();
+ capture_worker_thread_.reset();
+ _critSect.Enter();
}
- _critSect.Enter();
WEBRTC_CA_LOG_WARN(AudioConverterDispose(_captureConverter));
@@ -1954,17 +1909,15 @@
return 0;
}
- OSStatus err = noErr;
-
- unsigned int threadID(0);
- if (_renderWorkerThread != NULL)
- {
- _renderWorkerThread->Start(threadID);
- }
- _renderWorkerThreadId = threadID;
+ DCHECK(!render_worker_thread_.get());
+ render_worker_thread_.reset(
+ ThreadWrapper::CreateThread(RunRender, this, kRealtimePriority,
+ "RenderWorkerThread"));
+ render_worker_thread_->Start(render_worker_thread_id_);
if (_twoDevices || !_recording)
{
+ OSStatus err = noErr;
WEBRTC_CA_RETURN_ON_ERR(AudioDeviceStart(_outputDeviceID, _deviceIOProcID));
}
_playing = true;
@@ -2019,17 +1972,12 @@
// Setting this signal will allow the worker thread to be stopped.
AtomicSet32(&_renderDeviceIsAlive, 0);
- _critSect.Leave();
- if (_renderWorkerThread != NULL)
- {
- if (!_renderWorkerThread->Stop())
- {
- WEBRTC_TRACE(kTraceError, kTraceAudioDevice, _id,
- " Timed out waiting for the render worker thread to "
- "stop.");
- }
+ if (render_worker_thread_.get()) {
+ _critSect.Leave();
+ render_worker_thread_->Stop();
+ render_worker_thread_.reset();
+ _critSect.Enter();
}
- _critSect.Enter();
WEBRTC_CA_LOG_WARN(AudioConverterDispose(_renderConverter));
@@ -2483,7 +2431,7 @@
void* clientData)
{
AudioDeviceMac *ptrThis = (AudioDeviceMac *) clientData;
- assert(ptrThis != NULL);
+ DCHECK(ptrThis != NULL);
ptrThis->implObjectListenerProc(objectId, numberAddresses, addresses);
@@ -2791,7 +2739,7 @@
void *clientData)
{
AudioDeviceMac *ptrThis = (AudioDeviceMac *) clientData;
- assert(ptrThis != NULL);
+ DCHECK(ptrThis != NULL);
ptrThis->implDeviceIOProc(inputData, inputTime, outputData, outputTime);
@@ -2806,7 +2754,7 @@
void *userData)
{
AudioDeviceMac *ptrThis = (AudioDeviceMac *) userData;
- assert(ptrThis != NULL);
+ DCHECK(ptrThis != NULL);
return ptrThis->implOutConverterProc(numberDataPackets, data);
}
@@ -2818,7 +2766,7 @@
const AudioTimeStamp*, void* clientData)
{
AudioDeviceMac *ptrThis = (AudioDeviceMac *) clientData;
- assert(ptrThis != NULL);
+ DCHECK(ptrThis != NULL);
ptrThis->implInDeviceIOProc(inputData, inputTime);
@@ -2834,7 +2782,7 @@
void *userData)
{
AudioDeviceMac *ptrThis = static_cast<AudioDeviceMac*> (userData);
- assert(ptrThis != NULL);
+ DCHECK(ptrThis != NULL);
return ptrThis->implInConverterProc(numberDataPackets, data);
}
@@ -2891,7 +2839,7 @@
return 0;
}
- assert(_outStreamFormat.mBytesPerFrame != 0);
+ DCHECK(_outStreamFormat.mBytesPerFrame != 0);
UInt32 size = outputData->mBuffers->mDataByteSize
/ _outStreamFormat.mBytesPerFrame;
@@ -2932,7 +2880,7 @@
OSStatus AudioDeviceMac::implOutConverterProc(UInt32 *numberDataPackets,
AudioBufferList *data)
{
- assert(data->mNumberBuffers == 1);
+ DCHECK(data->mNumberBuffers == 1);
PaRingBufferSize numSamples = *numberDataPackets
* _outDesiredFormat.mChannelsPerFrame;
@@ -3006,7 +2954,7 @@
AtomicSet32(&_captureDelayUs, captureDelayUs);
- assert(inputData->mNumberBuffers == 1);
+ DCHECK(inputData->mNumberBuffers == 1);
PaRingBufferSize numSamples = inputData->mBuffers->mDataByteSize
* _inStreamFormat.mChannelsPerFrame / _inStreamFormat.mBytesPerPacket;
PaUtil_WriteRingBuffer(_paCaptureBuffer, inputData->mBuffers->mData,
@@ -3025,7 +2973,7 @@
OSStatus AudioDeviceMac::implInConverterProc(UInt32 *numberDataPackets,
AudioBufferList *data)
{
- assert(data->mNumberBuffers == 1);
+ DCHECK(data->mNumberBuffers == 1);
PaRingBufferSize numSamples = *numberDataPackets
* _inStreamFormat.mChannelsPerFrame;
diff --git a/webrtc/modules/audio_device/mac/audio_device_mac.h b/webrtc/modules/audio_device/mac/audio_device_mac.h
index 31e4fc1..e64b564 100644
--- a/webrtc/modules/audio_device/mac/audio_device_mac.h
+++ b/webrtc/modules/audio_device/mac/audio_device_mac.h
@@ -11,6 +11,7 @@
#ifndef WEBRTC_AUDIO_DEVICE_AUDIO_DEVICE_MAC_H
#define WEBRTC_AUDIO_DEVICE_AUDIO_DEVICE_MAC_H
+#include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/thread_annotations.h"
#include "webrtc/modules/audio_device/audio_device_generic.h"
#include "webrtc/modules/audio_device/mac/audio_mixer_manager_mac.h"
@@ -292,10 +293,13 @@
EventWrapper& _stopEventRec;
EventWrapper& _stopEvent;
- ThreadWrapper* _captureWorkerThread;
- ThreadWrapper* _renderWorkerThread;
- uint32_t _captureWorkerThreadId;
- uint32_t _renderWorkerThreadId;
+ // Only valid/running between calls to StartRecording and StopRecording.
+ rtc::scoped_ptr<ThreadWrapper> capture_worker_thread_;
+ unsigned int capture_worker_thread_id_;
+
+ // Only valid/running between calls to StartPlayout and StopPlayout.
+ rtc::scoped_ptr<ThreadWrapper> render_worker_thread_;
+ unsigned int render_worker_thread_id_;
int32_t _id;
diff --git a/webrtc/system_wrappers/source/thread.cc b/webrtc/system_wrappers/source/thread.cc
index 6f023f8..8d3fb40 100644
--- a/webrtc/system_wrappers/source/thread.cc
+++ b/webrtc/system_wrappers/source/thread.cc
@@ -24,7 +24,7 @@
#if defined(_WIN32)
return new ThreadWindows(func, obj, prio, thread_name);
#else
- return ThreadPosix::Create(func, obj, prio, thread_name);
+ return new ThreadPosix(func, obj, prio, thread_name);
#endif
}
diff --git a/webrtc/system_wrappers/source/thread_posix.cc b/webrtc/system_wrappers/source/thread_posix.cc
index e7fe2fe..0ddf17c 100644
--- a/webrtc/system_wrappers/source/thread_posix.cc
+++ b/webrtc/system_wrappers/source/thread_posix.cc
@@ -8,47 +8,11 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-// The state of a thread is controlled by the two member variables
-// alive_ and dead_.
-// alive_ represents the state the thread has been ordered to achieve.
-// It is set to true by the thread at startup, and is set to false by
-// other threads, using SetNotAlive() and Stop().
-// dead_ represents the state the thread has achieved.
-// It is written by the thread encapsulated by this class only
-// (except at init). It is read only by the Stop() method.
-// The Run() method fires event_ when it's started; this ensures that the
-// Start() method does not continue until after dead_ is false.
-// This protects against premature Stop() calls from the creator thread, but
-// not from other threads.
-
-// Their transitions and states:
-// alive_ dead_ Set by
-// false true Constructor
-// true false Run() method entry
-// false any Run() method run_function failure
-// any false Run() method exit (happens only with alive_ false)
-// false any SetNotAlive
-// false any Stop Stop waits for dead_ to become true.
-//
-// Summarized a different way:
-// Variable Writer Reader
-// alive_ Constructor(false) Run.loop
-// Run.start(true)
-// Run.fail(false)
-// SetNotAlive(false)
-// Stop(false)
-//
-// dead_ Constructor(true) Stop.loop
-// Run.start(false)
-// Run.exit(true)
-
#include "webrtc/system_wrappers/source/thread_posix.h"
#include <algorithm>
-#include <assert.h>
#include <errno.h>
-#include <string.h> // strncpy
#include <unistd.h>
#ifdef WEBRTC_LINUX
#include <linux/unistd.h>
@@ -58,16 +22,25 @@
#include <sys/types.h>
#endif
+#include "webrtc/base/checks.h"
#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
#include "webrtc/system_wrappers/interface/event_wrapper.h"
#include "webrtc/system_wrappers/interface/sleep.h"
#include "webrtc/system_wrappers/interface/trace.h"
namespace webrtc {
+namespace {
+struct ThreadAttributes {
+ ThreadAttributes() { pthread_attr_init(&attr); }
+ ~ThreadAttributes() { pthread_attr_destroy(&attr); }
+ pthread_attr_t* operator&() { return &attr; }
+ pthread_attr_t attr;
+};
+} // namespace
int ConvertToSystemPriority(ThreadPriority priority, int min_prio,
int max_prio) {
- assert(max_prio - min_prio > 2);
+ DCHECK(max_prio - min_prio > 2);
const int top_prio = max_prio - 1;
const int low_prio = min_prio + 1;
@@ -85,178 +58,95 @@
case kRealtimePriority:
return top_prio;
}
- assert(false);
+ DCHECK(false);
return low_prio;
}
-extern "C"
-{
- static void* StartThread(void* lp_parameter) {
- static_cast<ThreadPosix*>(lp_parameter)->Run();
- return 0;
+struct ThreadPosix::InitParams {
+ InitParams(ThreadPosix* thread)
+ : me(thread), started(EventWrapper::Create()) {
}
-}
+ ThreadPosix* me;
+ rtc::scoped_ptr<EventWrapper> started;
+};
-ThreadWrapper* ThreadPosix::Create(ThreadRunFunction func, ThreadObj obj,
- ThreadPriority prio,
- const char* thread_name) {
- ThreadPosix* ptr = new ThreadPosix(func, obj, prio, thread_name);
- if (!ptr) {
- return NULL;
- }
- const int error = ptr->Construct();
- if (error) {
- delete ptr;
- return NULL;
- }
- return ptr;
+// static
+void* ThreadPosix::StartThread(void* param) {
+ auto params = static_cast<InitParams*>(param);
+ params->me->Run(params);
+ return 0;
}
ThreadPosix::ThreadPosix(ThreadRunFunction func, ThreadObj obj,
ThreadPriority prio, const char* thread_name)
: run_function_(func),
obj_(obj),
- crit_state_(CriticalSectionWrapper::CreateCriticalSection()),
- alive_(false),
- dead_(true),
prio_(prio),
- event_(EventWrapper::Create()),
- name_(),
- set_thread_name_(false),
-#if (defined(WEBRTC_LINUX) || defined(WEBRTC_ANDROID))
- pid_(-1),
-#endif
- attr_(),
+ stop_event_(EventWrapper::Create()),
+ name_(thread_name ? thread_name : "webrtc"),
+ thread_id_(0),
thread_(0) {
- if (thread_name != NULL) {
- set_thread_name_ = true;
- strncpy(name_, thread_name, kThreadMaxNameLength);
- name_[kThreadMaxNameLength - 1] = '\0';
- }
+ DCHECK(name_.length() < kThreadMaxNameLength);
}
uint32_t ThreadWrapper::GetThreadId() {
-#if defined(WEBRTC_ANDROID) || defined(WEBRTC_LINUX)
- return static_cast<uint32_t>(syscall(__NR_gettid));
-#elif defined(WEBRTC_MAC) || defined(WEBRTC_IOS)
- return pthread_mach_thread_np(pthread_self());
-#else
- return reinterpret_cast<uint32_t>(pthread_self());
-#endif
-}
-
-int ThreadPosix::Construct() {
- int result = 0;
-#if !defined(WEBRTC_ANDROID)
- // Enable immediate cancellation if requested, see Shutdown().
- result = pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
- if (result != 0) {
- return -1;
- }
- result = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
- if (result != 0) {
- return -1;
- }
-#endif
- result = pthread_attr_init(&attr_);
- if (result != 0) {
- return -1;
- }
- return 0;
+ return rtc::CurrentThreadId();
}
ThreadPosix::~ThreadPosix() {
- pthread_attr_destroy(&attr_);
- delete event_;
- delete crit_state_;
+ DCHECK(thread_checker_.CalledOnValidThread());
}
-#define HAS_THREAD_ID !defined(WEBRTC_IOS) && !defined(WEBRTC_MAC)
+bool ThreadPosix::Start(unsigned int& thread_id) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ DCHECK(!thread_id_) << "Thread already started?";
-bool ThreadPosix::Start(unsigned int& thread_id)
-{
- int result = pthread_attr_setdetachstate(&attr_, PTHREAD_CREATE_DETACHED);
+ ThreadAttributes attr;
// Set the stack stack size to 1M.
- result |= pthread_attr_setstacksize(&attr_, 1024 * 1024);
- event_->Reset();
- // If pthread_create was successful, a thread was created and is running.
- // Don't return false if it was successful since if there are any other
- // failures the state will be: thread was started but not configured as
- // asked for. However, the caller of this API will assume that a false
- // return value means that the thread never started.
- result |= pthread_create(&thread_, &attr_, &StartThread, this);
- if (result != 0) {
+ pthread_attr_setstacksize(&attr, 1024 * 1024);
+
+ InitParams params(this);
+ int result = pthread_create(&thread_, &attr, &StartThread, ¶ms);
+ if (result != 0)
return false;
- }
- {
- CriticalSectionScoped cs(crit_state_);
- dead_ = false;
- }
- // Wait up to 10 seconds for the OS to call the callback function. Prevents
- // race condition if Stop() is called too quickly after start.
- if (kEventSignaled != event_->Wait(WEBRTC_EVENT_10_SEC)) {
- WEBRTC_TRACE(kTraceError, kTraceUtility, -1,
- "posix thread event never triggered");
- // Timed out. Something went wrong.
- return true;
- }
+ CHECK_EQ(kEventSignaled, params.started->Wait(WEBRTC_EVENT_INFINITE));
+ DCHECK_NE(thread_id_, 0);
-#if HAS_THREAD_ID
- thread_id = static_cast<unsigned int>(thread_);
-#endif
+ thread_id = thread_id_;
+
return true;
}
void ThreadPosix::SetNotAlive() {
- CriticalSectionScoped cs(crit_state_);
- alive_ = false;
+ DCHECK(thread_checker_.CalledOnValidThread());
}
bool ThreadPosix::Stop() {
- bool dead = false;
- {
- CriticalSectionScoped cs(crit_state_);
- alive_ = false;
- dead = dead_;
- }
-
- // TODO(hellner) why not use an event here?
- // Wait up to 10 seconds for the thread to terminate
- for (int i = 0; i < 1000 && !dead; ++i) {
- SleepMs(10);
- {
- CriticalSectionScoped cs(crit_state_);
- dead = dead_;
- }
- }
- if (dead) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ if (!thread_id_)
return true;
- } else {
- return false;
- }
+
+ stop_event_->Set();
+ CHECK_EQ(0, pthread_join(thread_, nullptr));
+ thread_id_ = 0;
+ stop_event_->Reset();
+
+ return true;
}
-void ThreadPosix::Run() {
- {
- CriticalSectionScoped cs(crit_state_);
- alive_ = true;
- }
-#if (defined(WEBRTC_LINUX) || defined(WEBRTC_ANDROID))
- pid_ = GetThreadId();
-#endif
- // The event the Start() is waiting for.
- event_->Set();
+void ThreadPosix::Run(ThreadPosix::InitParams* params) {
+ thread_id_ = rtc::CurrentThreadId();
+ params->started->Set();
- if (set_thread_name_) {
-#ifdef WEBRTC_LINUX
- prctl(PR_SET_NAME, (unsigned long)name_, 0, 0, 0);
+ if (!name_.empty()) {
+ // Setting the thread name may fail (harmlessly) if running inside a
+ // sandbox. Ignore failures if they happen.
+#if defined(WEBRTC_LINUX) || defined(WEBRTC_ANDROID)
+ prctl(PR_SET_NAME, reinterpret_cast<unsigned long>(name_.c_str()));
+#elif defined(WEBRTC_MAC) || defined(WEBRTC_IOS)
+ pthread_setname_np(name_.substr(0, 63).c_str());
#endif
- WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1,
- "Thread with name:%s started ", name_);
- } else {
- WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1,
- "Thread without name started");
}
#ifdef WEBRTC_THREAD_RR
@@ -270,6 +160,7 @@
WEBRTC_TRACE(kTraceError, kTraceUtility, -1,
"unable to retreive min or max priority for threads");
}
+
if (max_prio - min_prio > 2) {
sched_param param;
param.sched_priority = ConvertToSystemPriority(prio_, min_prio, max_prio);
@@ -279,33 +170,13 @@
}
}
- bool alive = true;
- bool run = true;
- while (alive) {
- run = run_function_(obj_);
- CriticalSectionScoped cs(crit_state_);
- if (!run) {
- alive_ = false;
- }
- alive = alive_;
- }
-
- if (set_thread_name_) {
- // Don't set the name for the trace thread because it may cause a
- // deadlock. TODO(hellner) there should be a better solution than
- // coupling the thread and the trace class like this.
- if (strcmp(name_, "Trace")) {
- WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1,
- "Thread with name:%s stopped", name_);
- }
- } else {
- WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1,
- "Thread without name stopped");
- }
- {
- CriticalSectionScoped cs(crit_state_);
- dead_ = true;
- }
+ // It's a requirement that for successful thread creation that the run
+ // function be called at least once (see RunFunctionIsCalled unit test),
+ // so to fullfill that requirement, we use a |do| loop and not |while|.
+ do {
+ if (!run_function_(obj_))
+ break;
+ } while (stop_event_->Wait(0) == kEventTimeout);
}
} // namespace webrtc
diff --git a/webrtc/system_wrappers/source/thread_posix.h b/webrtc/system_wrappers/source/thread_posix.h
index 65e6da8..cd42c33 100644
--- a/webrtc/system_wrappers/source/thread_posix.h
+++ b/webrtc/system_wrappers/source/thread_posix.h
@@ -11,6 +11,8 @@
#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_THREAD_POSIX_H_
#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_THREAD_POSIX_H_
+#include "webrtc/base/scoped_ptr.h"
+#include "webrtc/base/thread_checker.h"
#include "webrtc/system_wrappers/interface/thread_wrapper.h"
#include <pthread.h>
@@ -25,44 +27,31 @@
class ThreadPosix : public ThreadWrapper {
public:
- static ThreadWrapper* Create(ThreadRunFunction func, ThreadObj obj,
- ThreadPriority prio, const char* thread_name);
-
ThreadPosix(ThreadRunFunction func, ThreadObj obj, ThreadPriority prio,
const char* thread_name);
- virtual ~ThreadPosix();
+ ~ThreadPosix() override;
// From ThreadWrapper.
- virtual void SetNotAlive() OVERRIDE;
- virtual bool Start(unsigned int& id) OVERRIDE;
- virtual bool Stop() OVERRIDE;
-
- void Run();
+ void SetNotAlive() override;
+ bool Start(unsigned int& id) override;
+ bool Stop() override;
private:
- int Construct();
+ static void* StartThread(void* param);
- private:
- ThreadRunFunction run_function_;
- ThreadObj obj_;
+ struct InitParams;
+ void Run(InitParams* params);
- // Internal state.
- CriticalSectionWrapper* crit_state_; // Protects alive_ and dead_
- bool alive_;
- bool dead_;
- ThreadPriority prio_;
- EventWrapper* event_;
+ rtc::ThreadChecker thread_checker_;
+ ThreadRunFunction const run_function_;
+ void* const obj_;
+ ThreadPriority prio_;
+ // TODO(tommi): std::condition_variable?
+ const rtc::scoped_ptr<EventWrapper> stop_event_;
+ const std::string name_;
- // Zero-terminated thread name string.
- char name_[kThreadMaxNameLength];
- bool set_thread_name_;
-
- // Handle to thread.
-#if (defined(WEBRTC_LINUX) || defined(WEBRTC_ANDROID))
- pid_t pid_;
-#endif
- pthread_attr_t attr_;
- pthread_t thread_;
+ pid_t thread_id_;
+ pthread_t thread_;
};
} // namespace webrtc
