Index: base/threading/thread.h |
diff --git a/base/threading/thread.h b/base/threading/thread.h |
index f68154e12d25c8d85fbf02ea8ae1d7f7a546d0c6..45064240d76f8436ceb7ab51ff205be4646a8485 100644 |
--- a/base/threading/thread.h |
+++ b/base/threading/thread.h |
@@ -15,6 +15,7 @@ |
#include "base/macros.h" |
#include "base/message_loop/message_loop.h" |
#include "base/message_loop/timer_slack.h" |
+#include "base/sequence_checker.h" |
#include "base/single_thread_task_runner.h" |
#include "base/synchronization/lock.h" |
#include "base/synchronization/waitable_event.h" |
@@ -39,6 +40,10 @@ class RunLoop; |
// (1) Thread::CleanUp() |
// (2) MessageLoop::~MessageLoop |
// (3.b) MessageLoop::DestructionObserver::WillDestroyCurrentMessageLoop |
+// |
+// This API is not thread-safe: unless indicated otherwise its methods are only |
+// valid from the owning sequence (which is the one from which Start() is |
+// invoked, should it differ from the one on which it was constructed). |
class BASE_EXPORT Thread : PlatformThread::Delegate { |
public: |
struct BASE_EXPORT Options { |
@@ -51,10 +56,10 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { |
// Specifies the type of message loop that will be allocated on the thread. |
// This is ignored if message_pump_factory.is_null() is false. |
- MessageLoop::Type message_loop_type; |
+ MessageLoop::Type message_loop_type = MessageLoop::TYPE_DEFAULT; |
// Specifies timer slack for thread message loop. |
- TimerSlack timer_slack; |
+ TimerSlack timer_slack = TIMER_SLACK_NONE; |
// Used to create the MessagePump for the MessageLoop. The callback is Run() |
// on the thread. If message_pump_factory.is_null(), then a MessagePump |
@@ -65,10 +70,10 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { |
// Specifies the maximum stack size that the thread is allowed to use. |
// This does not necessarily correspond to the thread's initial stack size. |
// A value of 0 indicates that the default maximum should be used. |
- size_t stack_size; |
+ size_t stack_size = 0; |
// Specifies the initial thread priority. |
- ThreadPriority priority; |
+ ThreadPriority priority = ThreadPriority::NORMAL; |
}; |
// Constructor. |
@@ -159,14 +164,39 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { |
// NOTE: You must not call this MessageLoop's Quit method directly. Use |
// the Thread's Stop method instead. |
// |
- MessageLoop* message_loop() const { return message_loop_; } |
+ // In addition to this Thread's owning sequence, this can also safely be |
+ // called from the underlying thread itself. |
+ MessageLoop* message_loop() const { |
+ // This class doesn't provide synchronization around |message_loop_| and as |
+ // such only the owner should access it (and the underlying thread which |
+ // never sees it before it's set). In practice, many callers are coming from |
+ // unrelated threads but provide their own implicit (e.g. memory barriers |
+ // from task posting) or explicit (e.g. locks) synchronization making the |
+ // access of |message_loop_| safe... Changing all of those callers is |
+ // unfeasible; instead verify that they can reliably see |
+ // |message_loop_ != nullptr| without synchronization as a proof that their |
+ // external synchronization catches the unsynchronized effects of Start(). |
+ // TODO(gab): Despite all of the above this test has to be disabled for now |
+ // per crbug.com/629139#c6. |
+ // DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread() || |
+ // id_ == PlatformThread::CurrentId() || message_loop_) |
+ // << id_ << " vs " << PlatformThread::CurrentId(); |
+ return message_loop_; |
+ } |
// Returns a TaskRunner for this thread. Use the TaskRunner's PostTask |
// methods to execute code on the thread. Returns nullptr if the thread is not |
// running (e.g. before Start or after Stop have been called). Callers can |
// hold on to this even after the thread is gone; in this situation, attempts |
// to PostTask() will fail. |
+ // |
+ // In addition to this Thread's owning sequence, this can also safely be |
+ // called from the underlying thread itself. |
scoped_refptr<SingleThreadTaskRunner> task_runner() const { |
+ // Refer to the DCHECK and comment inside |message_loop()|. |
+ DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread() || |
+ id_ == PlatformThread::CurrentId() || message_loop_) |
+ << id_ << " vs " << PlatformThread::CurrentId(); |
return message_loop_ ? message_loop_->task_runner() : nullptr; |
} |
@@ -179,6 +209,7 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { |
// |
// WARNING: This function will block if the thread hasn't started yet. |
// |
+ // This method is thread-safe. |
PlatformThreadId GetThreadId() const; |
// Returns true if the thread has been started, and not yet stopped. |
@@ -198,6 +229,7 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { |
static bool GetThreadWasQuitProperly(); |
void set_message_loop(MessageLoop* message_loop) { |
+ DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread()); |
message_loop_ = message_loop; |
} |
@@ -217,17 +249,17 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { |
#if defined(OS_WIN) |
// Whether this thread needs to initialize COM, and if so, in what mode. |
- ComStatus com_status_; |
+ ComStatus com_status_ = NONE; |
#endif |
// If true, we're in the middle of stopping, and shouldn't access |
// |message_loop_|. It may non-nullptr and invalid. |
// Should be written on the thread that created this thread. Also read data |
// could be wrong on other threads. |
- bool stopping_; |
+ bool stopping_ = false; |
// True while inside of Run(). |
- bool running_; |
+ bool running_ = false; |
mutable base::Lock running_lock_; // Protects |running_|. |
// The thread's handle. |
@@ -235,24 +267,28 @@ class BASE_EXPORT Thread : PlatformThread::Delegate { |
mutable base::Lock thread_lock_; // Protects |thread_|. |
// The thread's id once it has started. |
- PlatformThreadId id_; |
+ PlatformThreadId id_ = kInvalidThreadId; |
mutable WaitableEvent id_event_; // Protects |id_|. |
// The thread's MessageLoop and RunLoop. Valid only while the thread is alive. |
// Set by the created thread. |
- MessageLoop* message_loop_; |
- RunLoop* run_loop_; |
+ MessageLoop* message_loop_ = nullptr; |
+ RunLoop* run_loop_ = nullptr; |
// Stores Options::timer_slack_ until the message loop has been bound to |
// a thread. |
- TimerSlack message_loop_timer_slack_; |
+ TimerSlack message_loop_timer_slack_ = TIMER_SLACK_NONE; |
// The name of the thread. Used for debugging purposes. |
- std::string name_; |
+ const std::string name_; |
// Signaled when the created thread gets ready to use the message loop. |
mutable WaitableEvent start_event_; |
+ // This class is not thread-safe, use this to verify access from the owning |
+ // sequence of the Thread. |
+ SequenceChecker owning_sequence_checker_; |
+ |
DISALLOW_COPY_AND_ASSIGN(Thread); |
}; |