Commit 74d30d0b authored by Ken Rockot's avatar Ken Rockot Committed by Commit Bot

Avoid duplicate instance stopping at Service Manager shutdown

Instances are explicitly stopped during Service Manager shutdown, but
then "stopped" again during individual Instance destruction. That
results in listeners being notified twice about each instance being
stopped if they aren't stopped before Service Manager shutdown.

This CL fixes that.

Bug: 831386
Change-Id: I210c9ce07cfa386d39bba7aa42244e00519b6f81
Reviewed-on: https://chromium-review.googlesource.com/1008968Reviewed-by: default avatarJay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550179}
parent 51c1d401
...@@ -228,6 +228,8 @@ class ServiceManager::Instance ...@@ -228,6 +228,8 @@ class ServiceManager::Instance
} }
void Stop() { void Stop() {
DCHECK_NE(state_, State::STOPPED);
// Shutdown all bindings. This way the process should see the pipes closed // Shutdown all bindings. This way the process should see the pipes closed
// and exit, as well as waking up any potential // and exit, as well as waking up any potential
// sync/WaitForIncomingResponse(). // sync/WaitForIncomingResponse().
...@@ -244,10 +246,15 @@ class ServiceManager::Instance ...@@ -244,10 +246,15 @@ class ServiceManager::Instance
// Notify the ServiceManager that this Instance is really going away. // Notify the ServiceManager that this Instance is really going away.
service_manager_->OnInstanceStopped(identity_); service_manager_->OnInstanceStopped(identity_);
} }
state_ = State::STOPPED;
} }
~Instance() override { ~Instance() override {
Stop(); // The instance may have already been stopped prior to destruction if the
// ServiceManager itself is being torn down.
if (state_ != State::STOPPED)
Stop();
} }
bool CallOnBindInterface(std::unique_ptr<ConnectParams>* in_params) { bool CallOnBindInterface(std::unique_ptr<ConnectParams>* in_params) {
...@@ -373,7 +380,10 @@ class ServiceManager::Instance ...@@ -373,7 +380,10 @@ class ServiceManager::Instance
STARTING, STARTING,
// The service was started successfully. // The service was started successfully.
STARTED STARTED,
// The service has been stopped.
STOPPED,
}; };
class InterfaceProviderImpl : public mojom::InterfaceProvider { class InterfaceProviderImpl : public mojom::InterfaceProvider {
...@@ -897,10 +907,12 @@ ServiceManager::~ServiceManager() { ...@@ -897,10 +907,12 @@ ServiceManager::~ServiceManager() {
// the case where one Instance's destructor blocks waiting for its Runner to // 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 // quit, while that Runner's corresponding Service blocks its shutdown on a
// distinct Service receiving a connection error. // distinct Service receiving a connection error.
for (const auto& instance : instances_) for (const auto& instance : instances_) {
instance.first->Stop(); if (instance.first != service_manager_instance_)
service_manager_instance->Stop(); instance.first->Stop();
}
service_manager_instance->Stop();
instances_.clear(); instances_.clear();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment