Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(848)

Unified Diff: base/threading/simple_thread.cc

Issue 2664953004: Do not block in SimpleThread::Start(). (Closed)
Patch Set: CR gab #26 (comment) Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/threading/simple_thread.h ('k') | base/threading/simple_thread_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/threading/simple_thread.cc
diff --git a/base/threading/simple_thread.cc b/base/threading/simple_thread.cc
index 9eb443afab1b5d5dcb3dcd7427f8c4a1faacbb1b..65a56f212e269f4eef60cb65615126ed494bedd2 100644
--- a/base/threading/simple_thread.cc
+++ b/base/threading/simple_thread.cc
@@ -18,17 +18,25 @@ SimpleThread::SimpleThread(const std::string& name_prefix,
const Options& options)
: name_prefix_(name_prefix),
options_(options),
- event_(WaitableEvent::ResetPolicy::MANUAL,
- WaitableEvent::InitialState::NOT_SIGNALED) {}
+ id_event_(WaitableEvent::ResetPolicy::MANUAL,
+ WaitableEvent::InitialState::NOT_SIGNALED) {}
SimpleThread::~SimpleThread() {
- DCHECK(HasBeenStarted()) << "SimpleThread was never started.";
- DCHECK(!options_.joinable || HasBeenJoined())
+#if DCHECK_IS_ON()
+ DCHECK(has_been_started_) << "SimpleThread was never started.";
+ DCHECK(!options_.joinable || has_been_joined_)
<< "Joinable SimpleThread destroyed without being Join()ed.";
+#endif
}
void SimpleThread::Start() {
- DCHECK(!HasBeenStarted()) << "Tried to Start a thread multiple times.";
+#if DCHECK_IS_ON()
+ DCHECK(!has_been_started_) << "Tried to Start a thread multiple times.";
+
+ // Set |has_been_started_| before creating the thread as no member access is
+ // allowed after.
+ has_been_started_ = true;
+#endif
bool success =
options_.joinable
? PlatformThread::CreateWithPriority(options_.stack_size, this,
@@ -36,35 +44,39 @@ void SimpleThread::Start() {
: PlatformThread::CreateNonJoinableWithPriority(
options_.stack_size, this, options_.priority);
DCHECK(success);
- ThreadRestrictions::ScopedAllowWait allow_wait;
- event_.Wait(); // Wait for the thread to complete initialization.
+
+ // No member access after creating the thread, |this| can be deleted at any
+ // point after invoking Run() on non-joinable threads.
}
void SimpleThread::Join() {
+#if DCHECK_IS_ON()
DCHECK(options_.joinable) << "A non-joinable thread can't be joined.";
- DCHECK(HasBeenStarted()) << "Tried to Join a never-started thread.";
- DCHECK(!HasBeenJoined()) << "Tried to Join a thread multiple times.";
+ DCHECK(has_been_started_) << "Tried to Join a never-started thread.";
+ DCHECK(!has_been_joined_) << "Tried to Join a thread multiple times.";
+#endif
PlatformThread::Join(thread_);
thread_ = PlatformThreadHandle();
- joined_ = true;
+#if DCHECK_IS_ON()
+ has_been_joined_ = true;
+#endif
}
-bool SimpleThread::HasBeenStarted() {
- ThreadRestrictions::ScopedAllowWait allow_wait;
- return event_.IsSignaled();
+PlatformThreadId SimpleThread::GetTid() const {
+ id_event_.Wait();
+ return tid_;
}
void SimpleThread::ThreadMain() {
tid_ = PlatformThread::CurrentId();
+ id_event_.Signal();
+
// Construct our full name of the form "name_prefix_/TID".
std::string name(name_prefix_);
name.push_back('/');
name.append(IntToString(tid_));
PlatformThread::SetName(name);
- // We've initialized our new thread, signal that we're done to Start().
- event_.Signal();
-
Run();
}
« no previous file with comments | « base/threading/simple_thread.h ('k') | base/threading/simple_thread_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698