Fix a "benign" data race during thread construction

base/message_loop/message_loop.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/message_loop/message_loop.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 389 matching lines @@
   if (!pump_factory_.is_null())
     pump_ = pump_factory_.Run();
   else
     pump_ = CreateMessagePumpForType(type_);
 
   DCHECK(!current()) << "should only have one message loop per thread";
   lazy_tls_ptr.Pointer()->Set(this);
 
   incoming_task_queue_->StartScheduling();
   unbound_task_runner_->BindToCurrentThread();
-  SetTaskRunner(unbound_task_runner_.Pass());
+  unbound_task_runner_ = nullptr;

[danakj, 2015/07/10 18:32:38]

I'm feeling like this unbound_task_runner_ is providing less value and more
confusion over time.

What is is doing more than a bool bound_to_thread_ that only exists when
DCHECK_IS_ON()?

[Sami, 2015/07/13 10:43:31]

Yes, it's not all that useful but I can't think of a simpler way to do this. A
bool won't cut it because we still need an object on which we can call
BindToCurrentThread(). If we changed the type of task_runner_ to something that
has BindToCurrentThread(), then we'd need to leak that interface to the outside
world in SetTaskRunner.

The only other way I can think of is to keep task_runner_ a
SingleThreadTaskRunner and then do a downcast in BindToCurrentThread and add the
debug-only boolean. WDYT?

[danakj, 2015/07/13 18:33:47]

Ah, I see. LGTM then.

+  SetThreadTaskRunnerHandle();
 }
 
 void MessageLoop::SetTaskRunner(
     scoped_refptr<SingleThreadTaskRunner> task_runner) {
   DCHECK_EQ(this, current());
   DCHECK(task_runner->BelongsToCurrentThread());
   DCHECK(!unbound_task_runner_);
   task_runner_ = task_runner.Pass();
+  SetThreadTaskRunnerHandle();
+}
+
+void MessageLoop::SetThreadTaskRunnerHandle() {
+  DCHECK_EQ(this, current());
   // Clear the previous thread task runner first because only one can exist at
   // a time.
   thread_task_runner_handle_.reset();
   thread_task_runner_handle_.reset(new ThreadTaskRunnerHandle(task_runner_));
 }
 
 void MessageLoop::RunHandler() {
   DCHECK_EQ(this, current());
 
   StartHistogrammer();
@@ skipping 310 matching lines @@
       persistent,
       mode,
       controller,
       delegate);
 }
 #endif
 
 #endif  // !defined(OS_NACL_SFI)
 
 }  // namespace base