Remove dependency on rtc::Event from rtc::Thread.
The problem with using an Event for the 'running_' flag is that a Wait() is necessary to check the value of the flag and calling Wait() means a synchronous system call. In chromium that can furthermore trigger checks on threads where IO/blocking operations aren't allowed.
Luckily, we don't really need this variable. Instead, I'm adding thread checks to make sure that the Thread class is used correctly and ensure that locking isn't needed for modifying state (no locks are there now). As long as we follow those rules, we only need to check if a thread_ variable has been set when we want to know if the thread is running or not.
Along the way, fixed some places where variables weren't being set or reset correctly.
Bug: webrtc:8596
Change-Id: I1467542416bc2ffbfefe276c460e76078a759bd9
Reviewed-on: https://webrtc-review.googlesource.com/27720
Reviewed-by: Henrik Grunell <henrikg@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21042}diff --git a/rtc_base/thread.h b/rtc_base/thread.h
index 3e22c63..d198ba8 100644
--- a/rtc_base/thread.h
+++ b/rtc_base/thread.h
@@ -22,9 +22,9 @@
#include <pthread.h>
#endif
#include "rtc_base/constructormagic.h"
-#include "rtc_base/event.h"
#include "rtc_base/messagequeue.h"
#include "rtc_base/platform_thread_types.h"
+#include "rtc_base/thread_checker.h"
#if defined(WEBRTC_WIN)
#include "rtc_base/win32.h"
@@ -71,11 +71,11 @@
#endif
#if defined(WEBRTC_WIN)
- DWORD key_;
+ const DWORD key_;
#endif
// The thread to potentially autowrap.
- PlatformThreadRef main_thread_ref_;
+ const PlatformThreadRef main_thread_ref_;
RTC_DISALLOW_COPY_AND_ASSIGN(ThreadManager);
};
@@ -202,13 +202,13 @@
// You cannot call Start on non-owned threads.
bool IsOwned();
- // Expose private method running() for tests.
+ // Expose private method IsRunning() for tests.
//
// DANGER: this is a terrible public API. Most callers that might want to
// call this likely do not have enough control/knowledge of the Thread in
// question to guarantee that the returned value remains true for the duration
// of whatever code is conditionally executing because of the return value!
- bool RunningForTest() { return running(); }
+ bool RunningForTest() { return IsRunning(); }
// Sets the per-thread allow-blocking-calls flag and returns the previous
// value. Must be called on this thread.
@@ -256,8 +256,8 @@
bool WrapCurrentWithThreadManager(ThreadManager* thread_manager,
bool need_synchronize_access);
- // Return true if the thread was started and hasn't yet stopped.
- bool running() { return running_.Wait(0); }
+ // Return true if the thread is currently running.
+ bool IsRunning();
// Processes received "Send" requests. If |source| is not null, only requests
// from |source| are processed, otherwise, all requests are processed.
@@ -273,19 +273,31 @@
std::list<_SendMessage> sendlist_;
std::string name_;
- Event running_; // Signalled means running.
+
+ // Used to check access to unguarded thread control state variables and ensure
+ // that control functions are called on the right thread.
+ // The |thread_checker_| might represent a 'parent' thread from which
+ // Start()/Stop() are called or it could represent the worker thread itself in
+ // case the thread is wrapped since the functions that modified the control
+ // state will in this case be called from that thread.
+ ThreadChecker thread_checker_;
#if defined(WEBRTC_POSIX)
- pthread_t thread_;
+ pthread_t thread_ RTC_ACCESS_ON(thread_checker_) = 0;
#endif
#if defined(WEBRTC_WIN)
- HANDLE thread_;
- DWORD thread_id_;
+ HANDLE thread_ RTC_ACCESS_ON(thread_checker_) = nullptr;
+ DWORD thread_id_ RTC_ACCESS_ON(thread_checker_) = 0;
#endif
- bool owned_;
- bool blocking_calls_allowed_; // By default set to |true|.
+ // Indicates whether or not ownership of the worker thread lies with
+ // this instance or not. (i.e. owned_ == !wrapped).
+ // Must only be modified when the worker thread is not running.
+ bool owned_ = true;
+
+ // Only touched from the worker thread itself.
+ bool blocking_calls_allowed_ = true;
friend class ThreadManager;