[Thread-safe] Apply base::Passed to WebThread::Task

content/child/webthread_impl.cc

 // Copyright 2014 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.
 
 // An implementation of WebThread in terms of base::MessageLoop and
 // base::Thread
 
 #include "content/child/webthread_impl.h"
 
 #include "base/bind.h"
@@ skipping 42 matching lines @@
   base::MessageLoop::current()->RemoveTaskObserver(iter->second);
   delete iter->second;
   task_observer_map_.erase(iter);
 }
 
 WebThreadImpl::WebThreadImpl(const char* name)
     : thread_(new base::Thread(name)) {
   thread_->Start();
 }
 
+// RunWebThreadTask takes the ownership of |task| from base::Closure and
+// deletes it on the first invocation of the closure for thread-safety.
+// base::Closure made from RunWebThreadTask is copyable but Closure::Run
+// should be called at most only once.
+// This is because WebThread::Task can contain RefPtr to a
+// thread-unsafe-reference-counted object (e.g. WorkerThreadTask can contain
+// RefPtr to WebKit's StringImpl), and if we don't delete |task| here,
+// it causes a race condition as follows:
+// [A] In task->run(), more RefPtr's to the refcounted object can be created,
+//     and the reference counter of the object can be modified via these
+//     RefPtr's (as intended) on the thread where the task is executed.
+// [B] However, base::Closure still retains the ownership of WebThread::Task
+//     even after RunWebThreadTask is called.
+//     When base::Closure is deleted, WebThread::Task is deleted and the
+//     reference counter of the object is decreased by one, possibly from a
+//     different thread from [A], which is a race condition.
+// Taking the ownership of |task| here by using scoped_ptr and base::Passed
+// removes the reference counter modification of [B] and the race condition.
+// When the closure never runs at all, the corresponding WebThread::Task is
+// destructed when base::Closure is deleted (like [B]). In this case, there
+// are no reference counter modification like [A] (because task->run() is not
+// executed), so there are no race conditions.
+// See https://crbug.com/390851 for more details.
+static void RunWebThreadTask(scoped_ptr<blink::WebThread::Task> task) {
+    task->run();
+}
+
 void WebThreadImpl::postTask(Task* task) {
   thread_->message_loop()->PostTask(
-      FROM_HERE, base::Bind(&blink::WebThread::Task::run, base::Owned(task)));
+      FROM_HERE,
+      base::Bind(RunWebThreadTask, base::Passed(scoped_ptr<Task>(task))));
 }
 
 void WebThreadImpl::postDelayedTask(Task* task, long long delay_ms) {
   thread_->message_loop()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&blink::WebThread::Task::run, base::Owned(task)),
+      base::Bind(RunWebThreadTask, base::Passed(scoped_ptr<Task>(task))),
       base::TimeDelta::FromMilliseconds(delay_ms));
 }
 
 void WebThreadImpl::enterRunLoop() {
   CHECK(isCurrentThread());
   CHECK(!thread_->message_loop()->is_running());  // We don't support nesting.
   thread_->message_loop()->Run();
 }
 
 void WebThreadImpl::exitRunLoop() {
@@ skipping 14 matching lines @@
   thread_->Stop();
 }
 
 WebThreadImplForMessageLoop::WebThreadImplForMessageLoop(
     scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner)
     : main_thread_task_runner_(main_thread_task_runner),
       thread_id_(base::PlatformThread::CurrentId()) {}
 
 void WebThreadImplForMessageLoop::postTask(Task* task) {
   main_thread_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&blink::WebThread::Task::run, base::Owned(task)));
+      FROM_HERE,
+      base::Bind(RunWebThreadTask, base::Passed(make_scoped_ptr(task))));
 }
 
 void WebThreadImplForMessageLoop::postDelayedTask(Task* task,
                                                   long long delay_ms) {
   main_thread_task_runner_->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&blink::WebThread::Task::run, base::Owned(task)),
+      base::Bind(RunWebThreadTask, base::Passed(make_scoped_ptr(task))),
       base::TimeDelta::FromMilliseconds(delay_ms));
 }
 
 void WebThreadImplForMessageLoop::enterRunLoop() {
   CHECK(isCurrentThread());
   // We don't support nesting.
   CHECK(!base::MessageLoop::current()->is_running());
   base::MessageLoop::current()->Run();
 }
 
 void WebThreadImplForMessageLoop::exitRunLoop() {
   CHECK(isCurrentThread());
   CHECK(base::MessageLoop::current()->is_running());
   base::MessageLoop::current()->Quit();
 }
 
 bool WebThreadImplForMessageLoop::isCurrentThread() const {
   return main_thread_task_runner_->BelongsToCurrentThread();
 }
 
 blink::PlatformThreadId WebThreadImplForMessageLoop::threadId() const {
   return thread_id_;
 }
 
 WebThreadImplForMessageLoop::~WebThreadImplForMessageLoop() {}
 
 }  // namespace content