Fix deadlock in sharing service

The sharing service does a bunch of blocking on various threads during
shutdown. This can interfere with normal Mojo SharedRemote operation
when the backing pipe is bound on such threads.

This CL moves the SharedRemote bindings to the IO thread so that these
blocking operations cannot interfere with IPC behavior and cause
deadlocks.

Bug: 1130069
Change-Id: I553d0d8de354afa125542ea49801eabd63cac507
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439440
Auto-Submit: Ken Rockot <rockot@google.com>
Reviewed-by: James Vecore <vecore@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#812054}

chrome/services/sharing/nearby/nearby_connections.cc

@@ -88,6 +88,7 @@ NearbyConnections& NearbyConnections::GetInstance() {
 NearbyConnections::NearbyConnections(
     mojo::PendingReceiver<mojom::NearbyConnections> nearby_connections,
     mojom::NearbyConnectionsDependenciesPtr dependencies,
+    scoped_refptr<base::SequencedTaskRunner> io_task_runner,
     base::OnceClosure on_disconnect,
     std::unique_ptr<Core> core)
     : nearby_connections_(this, std::move(nearby_connections)),
@@ -99,7 +100,7 @@ NearbyConnections::NearbyConnections(
 
   if (dependencies->bluetooth_adapter) {
     bluetooth_adapter_.Bind(std::move(dependencies->bluetooth_adapter),
-                            /*bind_task_runner=*/nullptr);
+                            io_task_runner);
     bluetooth_adapter_.set_disconnect_handler(
         base::BindOnce(&NearbyConnections::OnDisconnect,
                        weak_ptr_factory_.GetWeakPtr()),
@@ -108,7 +109,7 @@ NearbyConnections::NearbyConnections(
 
   socket_manager_.Bind(
       std::move(dependencies->webrtc_dependencies->socket_manager),
-      /*bind_task_runner=*/nullptr);
+      io_task_runner);
   socket_manager_.set_disconnect_handler(
       base::BindOnce(&NearbyConnections::OnDisconnect,
                      weak_ptr_factory_.GetWeakPtr()),
@@ -116,7 +117,7 @@ NearbyConnections::NearbyConnections(
 
   mdns_responder_.Bind(
       std::move(dependencies->webrtc_dependencies->mdns_responder),
-      /*bind_task_runner=*/nullptr);
+      io_task_runner);
   mdns_responder_.set_disconnect_handler(
       base::BindOnce(&NearbyConnections::OnDisconnect,
                      weak_ptr_factory_.GetWeakPtr()),
@@ -124,15 +125,14 @@ NearbyConnections::NearbyConnections(
 
   ice_config_fetcher_.Bind(
       std::move(dependencies->webrtc_dependencies->ice_config_fetcher),
-      /*bind_task_runner=*/nullptr);
+      io_task_runner);
   ice_config_fetcher_.set_disconnect_handler(
       base::BindOnce(&NearbyConnections::OnDisconnect,
                      weak_ptr_factory_.GetWeakPtr()),
       base::SequencedTaskRunnerHandle::Get());
 
   webrtc_signaling_messenger_.Bind(
-      std::move(dependencies->webrtc_dependencies->messenger),
-      /*bind_task_runner=*/nullptr);
+      std::move(dependencies->webrtc_dependencies->messenger), io_task_runner);
   webrtc_signaling_messenger_.set_disconnect_handler(
       base::BindOnce(&NearbyConnections::OnDisconnect,
                      weak_ptr_factory_.GetWeakPtr()),

chrome/services/sharing/nearby/nearby_connections.h

@@ -11,8 +11,10 @@
 #include "base/callback_forward.h"
 #include "base/containers/flat_map.h"
 #include "base/files/file.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
 #include "base/task/post_task.h"
@@ -46,6 +48,7 @@ class NearbyConnections : public mojom::NearbyConnections {
   NearbyConnections(
       mojo::PendingReceiver<mojom::NearbyConnections> nearby_connections,
       mojom::NearbyConnectionsDependenciesPtr dependencies,
+      scoped_refptr<base::SequencedTaskRunner> io_task_runner,
       base::OnceClosure on_disconnect,
       std::unique_ptr<Core> core = std::make_unique<Core>());
 

chrome/services/sharing/nearby/nearby_connections_unittest.cc

@@ -191,6 +191,7 @@ class NearbyConnectionsTest : public testing::Test {
     service_controller_ptr_ = service_controller_.get();
     nearby_connections_ = std::make_unique<NearbyConnections>(
         remote_.BindNewPipeAndPassReceiver(), std::move(dependencies),
+        /*io_task_runner=*/nullptr,
         base::BindOnce(&NearbyConnectionsTest::OnDisconnect,
                        base::Unretained(this)),
         std::make_unique<Core>(

chrome/services/sharing/sharing_impl.cc

@@ -13,8 +13,11 @@
 
 namespace sharing {
 
-SharingImpl::SharingImpl(mojo::PendingReceiver<mojom::Sharing> receiver)
-    : receiver_(this, std::move(receiver)) {}
+SharingImpl::SharingImpl(
+    mojo::PendingReceiver<mojom::Sharing> receiver,
+    scoped_refptr<base::SequencedTaskRunner> io_task_runner)
+    : receiver_(this, std::move(receiver)),
+      io_task_runner_(std::move(io_task_runner)) {}
 
 SharingImpl::~SharingImpl() = default;
 
@@ -27,6 +30,7 @@ void SharingImpl::CreateNearbyConnections(
   mojo::PendingRemote<NearbyConnectionsMojom> remote;
   nearby_connections_ = std::make_unique<NearbyConnections>(
       remote.InitWithNewPipeAndPassReceiver(), std::move(dependencies),
+      io_task_runner_,
       base::BindOnce(&SharingImpl::NearbyConnectionsDisconnected,
                      weak_ptr_factory_.GetWeakPtr()));
   std::move(callback).Run(std::move(remote));

chrome/services/sharing/sharing_impl.h

@@ -11,6 +11,7 @@
 
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/sequenced_task_runner.h"
 #include "chrome/services/sharing/public/mojom/nearby_connections.mojom-forward.h"
 #include "chrome/services/sharing/public/mojom/sharing.mojom.h"
 #include "chrome/services/sharing/public/mojom/webrtc.mojom-forward.h"
@@ -40,7 +41,8 @@ class SharingImpl : public mojom::Sharing {
   using NearbyConnectionsDependenciesPtr =
       location::nearby::connections::mojom::NearbyConnectionsDependenciesPtr;
 
-  explicit SharingImpl(mojo::PendingReceiver<mojom::Sharing> receiver);
+  SharingImpl(mojo::PendingReceiver<mojom::Sharing> receiver,
+              scoped_refptr<base::SequencedTaskRunner> io_task_runner);
   SharingImpl(const SharingImpl&) = delete;
   SharingImpl& operator=(const SharingImpl&) = delete;
   ~SharingImpl() override;
@@ -56,6 +58,7 @@ class SharingImpl : public mojom::Sharing {
   void NearbyConnectionsDisconnected();
 
   mojo::Receiver<mojom::Sharing> receiver_;
+  const scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
 
   std::unique_ptr<NearbyConnections> nearby_connections_;
 

chrome/services/sharing/sharing_impl_unittest.cc

@@ -27,7 +27,8 @@ class SharingImplTest : public testing::Test {
 
   SharingImplTest() {
     service_ =
-        std::make_unique<SharingImpl>(remote_.BindNewPipeAndPassReceiver());
+        std::make_unique<SharingImpl>(remote_.BindNewPipeAndPassReceiver(),
+                                      /*io_task_runner=*/nullptr);
   }
 
   ~SharingImplTest() override {

chrome/utility/services.cc

@@ -231,7 +231,8 @@ auto RunImeService(
 }
 
 auto RunSharing(mojo::PendingReceiver<sharing::mojom::Sharing> receiver) {
-  return std::make_unique<sharing::SharingImpl>(std::move(receiver));
+  return std::make_unique<sharing::SharingImpl>(
+      std::move(receiver), content::UtilityThread::Get()->GetIOTaskRunner());
 }
 
 auto RunTtsService(