generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Rewrite ThreadWindows.

Browse Source 
* Remove "dead" and "alive" variables.
* Remove critical section
* Skip synchronizing with the worker thread to verify startup (no need).
* Remove implementation of SetNotAlive()
* Always set thread name
* Add thread checks for correct usage.

Also added some TODOs for myself for the ThreadWrapper interface.

I'm removing the HasNoMonitorThread test since it is no longer relevant and ends up checking the wrong thing (ProcessThread - a generic thread type) in the wrong way (parsing a debug log) :)  I think it served a purpose some years ago, but things have changed since.

BUG=2902
R=henrika@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/37069004

Cr-Commit-Position: refs/heads/master@{#8220}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8220 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
tommi@webrtc.org 2015-01-30 15:06:10 +00:00
parent f2ec814e0f
commit aef0779dab
4 changed files with 88 additions and 176 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
talk/media/webrtc/webrtcvoiceengine_unittest.cc
View File

@ -3185,25 +3185,6 @@ TEST(WebRtcVoiceEngineTest, DISABLED_HasUnencryptedLogging) {
EXPECT_TRUE(cleartext);
}
// Tests we do not see any references to a monitor thread being spun up
// when initiating the engine.
TEST(WebRtcVoiceEngineTest, HasNoMonitorThread) {
cricket::WebRtcVoiceEngine engine;
rtc::scoped_ptr<rtc::MemoryStream> stream(
new rtc::MemoryStream);
rtc::LogMessage::AddLogToStream(stream.get(), rtc::LS_VERBOSE);
engine.SetLogging(rtc::LS_VERBOSE, "");
EXPECT_TRUE(engine.Init(rtc::Thread::Current()));
engine.Terminate();
rtc::LogMessage::RemoveLogToStream(stream.get());
size_t size = 0;
EXPECT_TRUE(stream->GetSize(&size));
EXPECT_GT(size, 0U);
const std::string logs(stream->GetBuffer(), size);
EXPECT_NE(std::string::npos, logs.find("ProcessThread"));
}
// Tests that the library is configured with the codecs we want.
TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) {
cricket::WebRtcVoiceEngine engine;

21
webrtc/system_wrappers/interface/thread_wrapper.h
View File

@ -23,12 +23,13 @@ namespace webrtc {
// Object that will be passed by the spawned thread when it enters the callback
// function.
// TODO(tommi): Remove this define.
#define ThreadObj void*
// Callback function that the spawned thread will enter once spawned.
// A return value of false is interpreted as that the function has no
// more work to do and that the thread can be released.
typedef bool(*ThreadRunFunction)(ThreadObj);
typedef bool(*ThreadRunFunction)(void*);
enum ThreadPriority {
kLowPriority = 1,
@ -38,11 +39,14 @@ enum ThreadPriority {
kRealtimePriority = 5
};
// Represents a simple worker thread. The implementation must be assumed
// to be single threaded, meaning that all methods of the class, must be
// called from the same thread, including instantiation.
class ThreadWrapper {
public:
enum {kThreadMaxNameLength = 64};
virtual ~ThreadWrapper() {};
virtual ~ThreadWrapper() {}
// Factory method. Constructor disabled.
//
@ -52,24 +56,29 @@ class ThreadWrapper {
// prio Thread priority. May require root/admin rights.
// thread_name NULL terminated thread name, will be visable in the Windows
// debugger.
// TODO(tommi): Remove the priority argument and provide a setter instead.
// TODO(tommi): Make thread_name non-optional (i.e. no default value).
static ThreadWrapper* CreateThread(ThreadRunFunction func,
ThreadObj obj,
void* obj,
ThreadPriority prio = kNormalPriority,
const char* thread_name = 0);
// Get the current thread's kernel thread ID.
// Get the current thread's thread ID.
// NOTE: This is a static method. It returns the id of the calling thread,
// *not* the id of the worker thread that a ThreadWrapper instance represents.
// TODO(tommi): Move outside of the ThreadWrapper class to avoid confusion.
static uint32_t GetThreadId();
// Non blocking termination of the spawned thread. Note that it is not safe
// to delete this class until the spawned thread has been reclaimed.
// TODO(tommi): Remove this method.
virtual void SetNotAlive() = 0;
// Tries to spawns a thread and returns true if that was successful.
// Additionally, it tries to set thread priority according to the priority
// from when CreateThread was called. However, failure to set priority will
// not result in a false return value.
// TODO(henrike): add a function for polling whether priority was set or
// not.
// TODO(tommi): Remove the id parameter.
virtual bool Start(unsigned int& id) = 0;
// Stops the spawned thread and waits for it to be reclaimed with a timeout

179
webrtc/system_wrappers/source/thread_win.cc
View File

@ -10,11 +10,11 @@
#include "webrtc/system_wrappers/source/thread_win.h"
#include <assert.h>
#include <process.h>
#include <stdio.h>
#include <windows.h>
#include "webrtc/base/checks.h"
#include "webrtc/system_wrappers/interface/trace.h"
#include "webrtc/system_wrappers/source/set_thread_name_win.h"
@ -22,162 +22,97 @@ namespace webrtc {
ThreadWindows::ThreadWindows(ThreadRunFunction func, ThreadObj obj,
ThreadPriority prio, const char* thread_name)
: ThreadWrapper(),
run_function_(func),
: run_function_(func),
obj_(obj),
alive_(false),
dead_(true),
do_not_close_handle_(false),
prio_(prio),
event_(NULL),
event_(CreateEvent(NULL, FALSE, FALSE, NULL)),
thread_(NULL),
id_(0),
name_(),
set_thread_name_(false) {
event_ = EventWrapper::Create();
critsect_stop_ = CriticalSectionWrapper::CreateCriticalSection();
if (thread_name != NULL) {
// Set the thread name to appear in the VS debugger.
set_thread_name_ = true;
strncpy(name_, thread_name, kThreadMaxNameLength);
}
name_(thread_name ? thread_name : "webrtc") {
DCHECK(func);
DCHECK(event_);
}
ThreadWindows::~ThreadWindows() {
#ifdef _DEBUG
assert(!alive_);
#endif
if (thread_) {
CloseHandle(thread_);
}
if (event_) {
delete event_;
}
if (critsect_stop_) {
delete critsect_stop_;
}
DCHECK(main_thread_.CalledOnValidThread());
DCHECK(!thread_);
CloseHandle(event_);
}
// static
uint32_t ThreadWrapper::GetThreadId() {
return GetCurrentThreadId();
}
unsigned int WINAPI ThreadWindows::StartThread(LPVOID lp_parameter) {
static_cast<ThreadWindows*>(lp_parameter)->Run();
// static
DWORD WINAPI ThreadWindows::StartThread(void* param) {
static_cast<ThreadWindows*>(param)->Run();
return 0;
}
bool ThreadWindows::Start(unsigned int& thread_id) {
if (!run_function_) {
bool ThreadWindows::Start(unsigned int& id) {
DCHECK(main_thread_.CalledOnValidThread());
DCHECK(!thread_);
// See bug 2902 for stack size.
DWORD thread_id;
thread_ = ::CreateThread(NULL, 0, &StartThread, this,
STACK_SIZE_PARAM_IS_A_RESERVATION, &thread_id);
if (!thread_ ) {
DCHECK(false) << "CreateThread failed";
return false;
}
do_not_close_handle_ = false;
// Set stack size to 1M
thread_ = (HANDLE)_beginthreadex(NULL, 1024 * 1024, StartThread, (void*)this,
0, &thread_id);
if (thread_ == NULL) {
return false;
id = thread_id;
if (prio_ != kNormalPriority) {
int priority = THREAD_PRIORITY_NORMAL;
switch (prio_) {
case kLowPriority:
priority = THREAD_PRIORITY_BELOW_NORMAL;
break;
case kHighPriority:
priority = THREAD_PRIORITY_ABOVE_NORMAL;
break;
case kHighestPriority:
priority = THREAD_PRIORITY_HIGHEST;
break;
case kRealtimePriority:
priority = THREAD_PRIORITY_TIME_CRITICAL;
break;
default:
break;
}
SetThreadPriority(thread_, priority);
}
id_ = thread_id;
event_->Wait(INFINITE);
switch (prio_) {
case kLowPriority:
SetThreadPriority(thread_, THREAD_PRIORITY_BELOW_NORMAL);
break;
case kNormalPriority:
SetThreadPriority(thread_, THREAD_PRIORITY_NORMAL);
break;
case kHighPriority:
SetThreadPriority(thread_, THREAD_PRIORITY_ABOVE_NORMAL);
break;
case kHighestPriority:
SetThreadPriority(thread_, THREAD_PRIORITY_HIGHEST);
break;
case kRealtimePriority:
SetThreadPriority(thread_, THREAD_PRIORITY_TIME_CRITICAL);
break;
};
return true;
}
void ThreadWindows::SetNotAlive() {
alive_ = false;
DCHECK(main_thread_.CalledOnValidThread());
}
bool ThreadWindows::Stop() {
critsect_stop_->Enter();
// Prevents the handle from being closed in ThreadWindows::Run()
do_not_close_handle_ = true;
alive_ = false;
bool signaled = false;
if (thread_ && !dead_) {
critsect_stop_->Leave();
// Wait up to 2 seconds for the thread to complete.
if (WAIT_OBJECT_0 == WaitForSingleObject(thread_, 2000)) {
signaled = true;
}
critsect_stop_->Enter();
}
DCHECK(main_thread_.CalledOnValidThread());
if (thread_) {
SetEvent(event_);
WaitForSingleObject(thread_, INFINITE);
CloseHandle(thread_);
thread_ = NULL;
thread_ = nullptr;
}
critsect_stop_->Leave();
if (dead_ || signaled) {
return true;
} else {
return false;
}
return true;
}
void ThreadWindows::Run() {
alive_ = true;
dead_ = false;
event_->Set();
// All tracing must be after event_->Set to avoid deadlock in Trace.
if (set_thread_name_) {
WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, id_,
"Thread with name:%s started ", name_);
SetThreadName(static_cast<DWORD>(-1), name_); // -1 == caller thread.
} else {
WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, id_,
"Thread without name started");
}
if (!name_.empty())
SetThreadName(static_cast<DWORD>(-1), name_.c_str());
do {
if (run_function_) {
if (!run_function_(obj_)) {
alive_ = false;
}
} else {
alive_ = false;
}
} while (alive_);
if (set_thread_name_) {
WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, id_,
"Thread with name:%s stopped", name_);
} else {
WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, id_,
"Thread without name stopped");
}
critsect_stop_->Enter();
if (thread_ && !do_not_close_handle_) {
HANDLE thread = thread_;
thread_ = NULL;
CloseHandle(thread);
}
dead_ = true;
critsect_stop_->Leave();
};
if (!run_function_(obj_))
break;
} while (WaitForSingleObject(event_, 0) == WAIT_TIMEOUT);
}
} // namespace webrtc

45
webrtc/system_wrappers/source/thread_win.h
View File

@ -15,47 +15,34 @@
#include <windows.h>
#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
#include "webrtc/system_wrappers/interface/event_wrapper.h"
#include "webrtc/base/thread_checker.h"
namespace webrtc {
class ThreadWindows : public ThreadWrapper {
public:
ThreadWindows(ThreadRunFunction func, ThreadObj obj, ThreadPriority prio,
ThreadWindows(ThreadRunFunction func, void* obj, ThreadPriority prio,
const char* thread_name);
virtual ~ThreadWindows();
~ThreadWindows() override;
virtual bool Start(unsigned int& id);
virtual bool Stop();
virtual void SetNotAlive();
static unsigned int WINAPI StartThread(LPVOID lp_parameter);
bool Start(unsigned int& id) override;
bool Stop() override;
void SetNotAlive() override;
protected:
virtual void Run();
void Run();
private:
ThreadRunFunction run_function_;
ThreadObj obj_;
bool alive_;
bool dead_;
// TODO(hellner)
// do_not_close_handle_ member seem pretty redundant. Should be able to remove
// it. Basically it should be fine to reclaim the handle when calling stop
// and in the destructor.
bool do_not_close_handle_;
ThreadPriority prio_;
EventWrapper* event_;
CriticalSectionWrapper* critsect_stop_;
HANDLE thread_;
unsigned int id_;
char name_[kThreadMaxNameLength];
bool set_thread_name_;
static DWORD WINAPI StartThread(void* param);
ThreadRunFunction const run_function_;
void* const obj_;
HANDLE event_; // Used to signal stoppage.
// TODO(tommi): Consider having a SetPriority method instead of this variable.
ThreadPriority prio_;
HANDLE thread_;
const std::string name_;
rtc::ThreadChecker main_thread_;
};
} // namespace webrtc