commit d7f09fade60b6c647c7846c98dfcf2dc49a32563
author Wez <wez@chromium.org> Tue Oct 31 01:46:06 2017 +0900
committer Qijiang Fan <fqj@google.com> Fri Jun 05 07:37:26 2020 +0900
tree 322c574c87eddef5374e19f87ba13bc5b73d755b
parent cf6064c54f1934887ba59b1a2dc8243a00d8564c

Fix ipc::ChannelMojo not to call GetWeakPtr() from multiple threads.

ipc::ChannelMojo calls GetWeakPtr() to bind WeakPtrs into helpers
returned by CreateThreadSafeChannel(), and in OnPipeError().

Since these calls can be made concurrently from different threads, they
should create a WeakPtr at initialization to use in those calls.

Bug: 
Change-Id: I8a86c4e7241314b3aa23dba9915c14d1cb078d31
Reviewed-on: https://chromium-review.googlesource.com/742498
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512513}

CrOS-Libchrome-Original-Commit: 888ef82fea43a33ac86c4b7e2bfc554184133757

ipc/ipc_channel_mojo.cc

diff --git a/ipc/ipc_channel_mojo.cc b/ipc/ipc_channel_mojo.cc
index 6471d83..80bd7f9 100644
--- a/ipc/ipc_channel_mojo.cc
+++ b/ipc/ipc_channel_mojo.cc
@@ -111,6 +111,7 @@
       pipe_(handle.get()),
       listener_(listener),
       weak_factory_(this) {
+  weak_ptr_ = weak_factory_.GetWeakPtr();
   bootstrap_ = MojoBootstrap::Create(std::move(handle), mode, ipc_task_runner);
 }
 
@@ -183,9 +184,8 @@
   if (task_runner_->RunsTasksInCurrentSequence()) {
     listener_->OnChannelError();
   } else {
-    task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&ChannelMojo::OnPipeError, weak_factory_.GetWeakPtr()));
+    task_runner_->PostTask(FROM_HERE,
+                           base::Bind(&ChannelMojo::OnPipeError, weak_ptr_));
   }
 }
 
@@ -230,10 +230,9 @@
 ChannelMojo::CreateThreadSafeChannel() {
   return std::make_unique<mojo::ThreadSafeForwarder<mojom::Channel>>(
       task_runner_,
-      base::Bind(&ChannelMojo::ForwardMessageFromThreadSafePtr,
-                 weak_factory_.GetWeakPtr()),
+      base::Bind(&ChannelMojo::ForwardMessageFromThreadSafePtr, weak_ptr_),
       base::Bind(&ChannelMojo::ForwardMessageWithResponderFromThreadSafePtr,
-                 weak_factory_.GetWeakPtr()),
+                 weak_ptr_),
       *bootstrap_->GetAssociatedGroup());
 }
 

ipc/ipc_channel_mojo.h

diff --git a/ipc/ipc_channel_mojo.h b/ipc/ipc_channel_mojo.h
index 61bc0db..d5aab3f 100644
--- a/ipc/ipc_channel_mojo.h
+++ b/ipc/ipc_channel_mojo.h
@@ -114,6 +114,8 @@
       const std::string& name,
       mojo::ScopedInterfaceEndpointHandle handle) override;
 
+  base::WeakPtr<ChannelMojo> weak_ptr_;
+
   // A TaskRunner which runs tasks on the ChannelMojo's owning thread.
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;