[ServiceManager] Eliminate parent-child relationship between services

services/service_manager/service_manager.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.
 
 #include "services/service_manager/service_manager.h"
 
 #include <stdint.h>
 
 #include <utility>
 
@@ skipping 83 matching lines @@
         control_binding_(this),
         state_(State::IDLE),
         weak_factory_(this) {
     if (identity_.name() == service_manager::mojom::kServiceName ||
         identity_.name() == catalog::mojom::kServiceName) {
       pid_ = base::Process::Current().Pid();
     }
     DCHECK_NE(mojom::kInvalidInstanceID, id_);
   }
 
-  ~Instance() override {
-    // Shutdown all bindings before we close the runner. This way the process
-    // should see the pipes closed and exit, as well as waking up any potential
+  void Stop() {
+    // Shutdown all bindings. This way the process should see the pipes closed
+    // and exit, as well as waking up any potential
     // sync/WaitForIncomingResponse().
     service_.reset();
     if (pid_receiver_binding_.is_bound())
       pid_receiver_binding_.Close();
     connectors_.CloseAllBindings();
     service_manager_bindings_.CloseAllBindings();
     service_manager_->OnInstanceUnreachable(this);
 
     if (state_ == State::STARTING) {
       service_manager_->NotifyServiceFailedToStart(identity_);
     } else {
       // Notify the ServiceManager that this Instance is really going away.
       service_manager_->OnInstanceStopped(identity_);
     }
+  }
+
+  ~Instance() override {
+    Stop();
 
     // Release |runner_| so that if we are called back to OnRunnerCompleted()
     // we know we're in the destructor.
     std::unique_ptr<NativeRunner> runner = std::move(runner_);
     runner.reset();
   }
 
-  Instance* parent() const { return parent_; }
-
-  void AddChild(std::unique_ptr<Instance> child) {
-    child->parent_ = this;
-    children_.insert(std::make_pair(child.get(), std::move(child)));
-  }
-
-  void RemoveChild(Instance* child) {
-    auto it = children_.find(child);
-    DCHECK(it != children_.end());
-
-    // Deletes |child|.
-    children_.erase(it);
-  }
-
   bool ConnectToService(std::unique_ptr<ConnectParams>* connect_params) {
     if (!service_.is_bound())
       return false;
 
     std::unique_ptr<ConnectParams> params(std::move(*connect_params));
     if (!params->connect_callback().is_null()) {
       params->connect_callback().Run(mojom::ConnectResult::SUCCEEDED,
                                      identity_.user_id());
     }
 
@@ skipping 313 matching lines @@
   const InterfaceProviderSpecMap interface_provider_specs_;
   const InterfaceProviderSpec empty_spec_;
   const bool allow_any_application_;
   std::unique_ptr<NativeRunner> runner_;
   mojom::ServicePtr service_;
   mojo::Binding<mojom::PIDReceiver> pid_receiver_binding_;
   mojo::BindingSet<mojom::Connector> connectors_;
   mojo::BindingSet<mojom::ServiceManager> service_manager_bindings_;
   mojo::AssociatedBinding<mojom::ServiceControl> control_binding_;
   base::ProcessId pid_ = base::kNullProcessId;
-  Instance* parent_ = nullptr;
-  InstanceMap children_;
   State state_;
 
   // The number of outstanding OnConnect requests which are in flight.
   int pending_service_connections_ = 0;
 
   base::WeakPtrFactory<Instance> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Instance);
 };
 
@@ skipping 72 matching lines @@
 
   if (catalog)
     InitCatalog(std::move(catalog));
 }
 
 ServiceManager::~ServiceManager() {
   // Ensure we tear down the ServiceManager instance last. This is to avoid
   // hitting bindings DCHECKs, since the ServiceManager or Catalog may at any
   // given time own in-flight responders for Instances' Connector requests.
   std::unique_ptr<Instance> service_manager_instance;
-  auto iter = root_instances_.find(service_manager_instance_);
-  DCHECK(iter != root_instances_.end());
+  auto iter = instances_.find(service_manager_instance_);
+  DCHECK(iter != instances_.end());
   service_manager_instance = std::move(iter->second);
 
-  root_instances_.clear();
+  // Stop all of the instances before destroying any of them. This ensures that
+  // all Services will receive connection errors and avoids possible deadlock in
+  // the case where one Instance's destructor blocks waiting for its Runner to
+  // quit, while that Runner's corresponding Service blocks its shutdown on a
+  // distinct Service receiving a connection error.
+  for (const auto& instance : instances_)
+    instance.first->Stop();
+  service_manager_instance->Stop();
+
+  instances_.clear();
 }
 
 void ServiceManager::SetServiceOverrides(
     std::unique_ptr<ServiceOverrides> overrides) {
   service_overrides_ = std::move(overrides);
 }
 
 void ServiceManager::SetInstanceQuitCallback(
     base::Callback<void(const Identity&)> callback) {
   instance_quit_callback_ = callback;
@@ skipping 50 matching lines @@
   identity_to_resolver_[identity] = std::move(resolver_ptr);
   return resolver;
 }
 
 void ServiceManager::OnInstanceError(Instance* instance) {
   // We never clean up the ServiceManager's own instance.
   if (instance == service_manager_instance_)
     return;
 
   EraseInstanceIdentity(instance);
-  if (instance->parent()) {
-    // Deletes |instance|.
-    instance->parent()->RemoveChild(instance);
-  } else {
-    auto it = root_instances_.find(instance);
-    DCHECK(it != root_instances_.end());
+  auto it = instances_.find(instance);
+  DCHECK(it != instances_.end());
 
-    // Deletes |instance|.
-    root_instances_.erase(it);
-  }
+  // Deletes |instance|.
+  instances_.erase(it);
 }
 
 void ServiceManager::OnInstanceUnreachable(Instance* instance) {
   // If an Instance becomes unreachable, new connection requests for this
   // identity will elicit a new Instance instantiation. The unreachable instance
   // remains alive.
   EraseInstanceIdentity(instance);
 }
 
 void ServiceManager::OnInstanceStopped(const Identity& identity) {
@@ skipping 97 matching lines @@
 
 ServiceManager::Instance* ServiceManager::CreateInstance(
     const Identity& source,
     const Identity& target,
     const InterfaceProviderSpecMap& specs) {
   CHECK(target.user_id() != mojom::kInheritUserID);
 
   auto instance = base::MakeUnique<Instance>(this, target, specs);
   Instance* raw_instance = instance.get();
 
-  Instance* source_instance = GetExistingInstance(source);
-  if (source_instance)
-    source_instance->AddChild(std::move(instance));
-  else
-    root_instances_.insert(std::make_pair(raw_instance, std::move(instance)));
+  instances_.insert(std::make_pair(raw_instance, std::move(instance)));
 
   // NOTE: |instance| has been passed elsewhere. Use |raw_instance| from this
   // point forward. It's safe for the extent of this method.
 
   auto result =
       identity_to_instance_.insert(std::make_pair(target, raw_instance));
   DCHECK(result.second);
 
   mojom::RunningServiceInfoPtr info = raw_instance->CreateRunningServiceInfo();
   listeners_.ForAllPtrs([&info](mojom::ServiceManagerListener* listener) {
@@ skipping 174 matching lines @@
   // Now that the instance has a Service, we can connect to it.
   bool connected = instance->ConnectToService(&params);
   DCHECK(connected);
 }
 
 base::WeakPtr<ServiceManager> ServiceManager::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 }  // namespace service_manager