diff --git a/webrtc/base/thread_checker_impl.cc b/webrtc/base/thread_checker_impl.cc index 48478e713..0b419d950 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 @@ PlatformThreadId CurrentThreadId() { #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 5f851c4d4..67adc6ebd 100644 --- a/webrtc/base/thread_checker_impl.h +++ b/webrtc/base/thread_checker_impl.h @@ -29,6 +29,9 @@ typedef DWORD PlatformThreadId; 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 8a8979393..3f39cdf05 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 -#include #include // OSAtomicCompareAndSwap() #include // mach_task_self() #include // sysctlbyname() @@ -56,8 +55,6 @@ namespace webrtc } \ } while(0) -#define ARRAY_SIZE(a) (sizeof(a)/sizeof((a)[0])) - enum { MaxNumberDevices = 64 @@ -94,8 +91,8 @@ void AudioDeviceMac::logCAMsg(const TraceLevel level, 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 @@ AudioDeviceMac::AudioDeviceMac(const int32_t id) : _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 @@ AudioDeviceMac::AudioDeviceMac(const int32_t id) : 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 @@ AudioDeviceMac::~AudioDeviceMac() 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 @@ int32_t AudioDeviceMac::Init() } } - 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 @@ int32_t AudioDeviceMac::Terminate() retVal = -1; } - _critSect.Leave(); - _isShutDown = true; _initialized = false; _outputDeviceIsSpecified = false; _inputDeviceIsSpecified = false; + _critSect.Leave(); + return retVal; } @@ -1104,6 +1064,7 @@ int16_t AudioDeviceMac::PlayoutDevices() int32_t AudioDeviceMac::SetPlayoutDevice(uint16_t index) { + CriticalSectionScoped lock(&_critSect); if (_playIsInitialized) { @@ -1287,7 +1248,6 @@ int32_t AudioDeviceMac::RecordingIsAvailable(bool& available) int32_t AudioDeviceMac::InitPlayout() { - CriticalSectionScoped lock(&_critSect); if (_playing) @@ -1794,15 +1754,14 @@ int32_t AudioDeviceMac::StartRecording() 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 @@ int32_t AudioDeviceMac::StopRecording() // 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 @@ int32_t AudioDeviceMac::StartPlayout() 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 @@ int32_t AudioDeviceMac::StopPlayout() // 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 @@ OSStatus AudioDeviceMac::objectListenerProc( void* clientData) { AudioDeviceMac *ptrThis = (AudioDeviceMac *) clientData; - assert(ptrThis != NULL); + DCHECK(ptrThis != NULL); ptrThis->implObjectListenerProc(objectId, numberAddresses, addresses); @@ -2791,7 +2739,7 @@ OSStatus AudioDeviceMac::deviceIOProc(AudioDeviceID, const AudioTimeStamp*, void *clientData) { AudioDeviceMac *ptrThis = (AudioDeviceMac *) clientData; - assert(ptrThis != NULL); + DCHECK(ptrThis != NULL); ptrThis->implDeviceIOProc(inputData, inputTime, outputData, outputTime); @@ -2806,7 +2754,7 @@ OSStatus AudioDeviceMac::outConverterProc(AudioConverterRef, void *userData) { AudioDeviceMac *ptrThis = (AudioDeviceMac *) userData; - assert(ptrThis != NULL); + DCHECK(ptrThis != NULL); return ptrThis->implOutConverterProc(numberDataPackets, data); } @@ -2818,7 +2766,7 @@ OSStatus AudioDeviceMac::inDeviceIOProc(AudioDeviceID, const AudioTimeStamp*, const AudioTimeStamp*, void* clientData) { AudioDeviceMac *ptrThis = (AudioDeviceMac *) clientData; - assert(ptrThis != NULL); + DCHECK(ptrThis != NULL); ptrThis->implInDeviceIOProc(inputData, inputTime); @@ -2834,7 +2782,7 @@ OSStatus AudioDeviceMac::inConverterProc( void *userData) { AudioDeviceMac *ptrThis = static_cast (userData); - assert(ptrThis != NULL); + DCHECK(ptrThis != NULL); return ptrThis->implInConverterProc(numberDataPackets, data); } @@ -2891,7 +2839,7 @@ OSStatus AudioDeviceMac::implDeviceIOProc(const AudioBufferList *inputData, return 0; } - assert(_outStreamFormat.mBytesPerFrame != 0); + DCHECK(_outStreamFormat.mBytesPerFrame != 0); UInt32 size = outputData->mBuffers->mDataByteSize / _outStreamFormat.mBytesPerFrame; @@ -2932,7 +2880,7 @@ OSStatus AudioDeviceMac::implDeviceIOProc(const AudioBufferList *inputData, OSStatus AudioDeviceMac::implOutConverterProc(UInt32 *numberDataPackets, AudioBufferList *data) { - assert(data->mNumberBuffers == 1); + DCHECK(data->mNumberBuffers == 1); PaRingBufferSize numSamples = *numberDataPackets * _outDesiredFormat.mChannelsPerFrame; @@ -3006,7 +2954,7 @@ OSStatus AudioDeviceMac::implInDeviceIOProc(const AudioBufferList *inputData, 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::implInDeviceIOProc(const AudioBufferList *inputData, 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 31e4fc195..e64b564d3 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 @@ private: 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 capture_worker_thread_; + unsigned int capture_worker_thread_id_; + + // Only valid/running between calls to StartPlayout and StopPlayout. + rtc::scoped_ptr 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 6f023f869..8d3fb4083 100644 --- a/webrtc/system_wrappers/source/thread.cc +++ b/webrtc/system_wrappers/source/thread.cc @@ -24,7 +24,7 @@ ThreadWrapper* ThreadWrapper::CreateThread(ThreadRunFunction func, #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 e7fe2fecf..0ddf17cda 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 -#include #include -#include // strncpy #include #ifdef WEBRTC_LINUX #include @@ -58,16 +22,25 @@ #include #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 @@ int ConvertToSystemPriority(ThreadPriority priority, int min_prio, case kRealtimePriority: return top_prio; } - assert(false); + DCHECK(false); return low_prio; } -extern "C" -{ - static void* StartThread(void* lp_parameter) { - static_cast(lp_parameter)->Run(); - return 0; +struct ThreadPosix::InitParams { + InitParams(ThreadPosix* thread) + : me(thread), started(EventWrapper::Create()) { } -} + ThreadPosix* me; + rtc::scoped_ptr 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(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(syscall(__NR_gettid)); -#elif defined(WEBRTC_MAC) || defined(WEBRTC_IOS) - return pthread_mach_thread_np(pthread_self()); -#else - return reinterpret_cast(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); + + thread_id = thread_id_; -#if HAS_THREAD_ID - thread_id = static_cast(thread_); -#endif 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(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 @@ void ThreadPosix::Run() { 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 @@ void ThreadPosix::Run() { } } - 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 65e6da8ba..cd42c33e2 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 @@ -25,44 +27,31 @@ int ConvertToSystemPriority(ThreadPriority priority, int min_prio, 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 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