[sessionmgr] Fix sessionmgr crash that happens on agent runner teardown
- Sessionmgr crashes in debug builds when printing |message| because
it is corrupted in AgentRunner::Teardown
- Corruption happens because |termination_callback_| execution destroys
AgentRunner along with the captures (the |message|)
- This fix is an alternative to 246372, it avoids putting the termination
callback on heap.
MF-163 #done
MF-171 #done
TESTED: run_modular_tests
Change-Id: Iab2acc84679f1d5ddd1a0b924d9fb2a319c1aebe
diff --git a/bin/sessionmgr/agent_runner/agent_context_impl.cc b/bin/sessionmgr/agent_runner/agent_context_impl.cc
index 0ec1887..641b932 100644
--- a/bin/sessionmgr/agent_runner/agent_context_impl.cc
+++ b/bin/sessionmgr/agent_runner/agent_context_impl.cc
@@ -37,9 +37,8 @@
InitializeCall(AgentContextImpl* const agent_context_impl,
fuchsia::sys::Launcher* const launcher,
fuchsia::modular::AppConfig agent_config)
- : Operation(
- "AgentContextImpl::InitializeCall", [] {},
- agent_context_impl->url_),
+ : Operation("AgentContextImpl::InitializeCall", [] {},
+ agent_context_impl->url_),
agent_context_impl_(agent_context_impl),
launcher_(launcher),
agent_config_(std::move(agent_config)) {}
@@ -280,18 +279,17 @@
fuchsia::auth::AppConfig app_config,
fidl::InterfaceHandle<fuchsia::auth::AuthenticationUIContext>
auth_ui_context,
- std::vector<::std::string> app_scopes,
- fidl::StringPtr user_profile_id, fidl::StringPtr auth_code,
- AuthorizeCallback callback) {
+ std::vector<::std::string> app_scopes, fidl::StringPtr user_profile_id,
+ fidl::StringPtr auth_code, AuthorizeCallback callback) {
FXL_LOG(ERROR) << "AgentContextImpl::Authorize() not supported from agent "
<< "context";
callback(fuchsia::auth::Status::INVALID_REQUEST, nullptr);
}
-void AgentContextImpl::GetAccessToken(
- fuchsia::auth::AppConfig app_config, std::string user_profile_id,
- std::vector<::std::string> app_scopes,
- GetAccessTokenCallback callback) {
+void AgentContextImpl::GetAccessToken(fuchsia::auth::AppConfig app_config,
+ std::string user_profile_id,
+ std::vector<::std::string> app_scopes,
+ GetAccessTokenCallback callback) {
FXL_CHECK(token_manager_);
FXL_DLOG(INFO) << "AgentContextImpl::GetAccessToken() invoked for user:"
@@ -353,12 +351,13 @@
}));
}
-void AgentContextImpl::StopForTeardown() {
+void AgentContextImpl::StopForTeardown(const std::function<void()>& callback) {
FXL_DLOG(INFO) << "AgentContextImpl::StopForTeardown() " << url_;
operation_queue_.Add(new StopCall(true /* is agent runner terminating? */,
- this, [this](bool stopped) {
+ this, [this, callback](bool stopped) {
FXL_DCHECK(stopped);
agent_runner_->RemoveAgent(url_);
+ callback();
// |this| is no longer valid at this
// point.
}));
diff --git a/bin/sessionmgr/agent_runner/agent_context_impl.h b/bin/sessionmgr/agent_runner/agent_context_impl.h
index be9c736..afb742c 100644
--- a/bin/sessionmgr/agent_runner/agent_context_impl.h
+++ b/bin/sessionmgr/agent_runner/agent_context_impl.h
@@ -49,7 +49,7 @@
// Stops the running agent, irrespective of whether there are active
// AgentControllers or outstanding tasks. Calls into
// |AgentRunner::RemoveAgent()| to remove itself.
- void StopForTeardown();
+ void StopForTeardown(const std::function<void()>& callback);
// Called by AgentRunner when a component wants to connect to this agent.
// Connections will pend until fuchsia::modular::Agent::Initialize() responds
diff --git a/bin/sessionmgr/agent_runner/agent_runner.cc b/bin/sessionmgr/agent_runner/agent_runner.cc
index ef38087..47cf6c2 100644
--- a/bin/sessionmgr/agent_runner/agent_runner.cc
+++ b/bin/sessionmgr/agent_runner/agent_runner.cc
@@ -65,7 +65,8 @@
// This is called when agents are done being removed
auto called = std::make_shared<bool>(false);
- auto cont = [this, called, callback](const bool from_timeout) {
+ auto termination_callback = [this, called,
+ callback](const bool from_timeout) {
if (*called) {
return;
}
@@ -79,19 +80,22 @@
callback();
};
- termination_callback_ = [cont] { cont(false); };
-
for (auto& it : running_agents_) {
// The running agent will call |AgentRunner::RemoveAgent()| to remove itself
// from the agent runner. When all agents are done being removed,
- // |RemoveAgent()| will call |termination_callback_|.
- it.second->StopForTeardown();
+ // |termination_callback| will be executed.
+ it.second->StopForTeardown([this, termination_callback]() {
+ if (running_agents_.empty()) {
+ termination_callback(/* from_timeout= */ false);
+ }
+ });
}
- auto cont_timeout = [cont] { cont(true); };
-
async::PostDelayedTask(async_get_default_dispatcher(),
- std::move(cont_timeout), kTeardownTimeout);
+ [termination_callback] {
+ termination_callback(/* from_timeout= */ true);
+ },
+ kTeardownTimeout);
}
void AgentRunner::EnsureAgentIsRunning(const std::string& agent_url,
@@ -188,9 +192,7 @@
void AgentRunner::RemoveAgent(const std::string agent_url) {
running_agents_.erase(agent_url);
- if (*terminating_ && running_agents_.empty()) {
- FXL_DCHECK(termination_callback_);
- termination_callback_();
+ if (*terminating_) {
return;
}
diff --git a/bin/sessionmgr/agent_runner/agent_runner.h b/bin/sessionmgr/agent_runner/agent_runner.h
index 48ddf28..ca734f0 100644
--- a/bin/sessionmgr/agent_runner/agent_runner.h
+++ b/bin/sessionmgr/agent_runner/agent_runner.h
@@ -218,9 +218,6 @@
// When this is marked true, no new new tasks will be scheduled.
std::shared_ptr<bool> terminating_;
- // This is called as part of the |StopForTeardown()| flow, when the last agent
- // is torn down.
- std::function<void()> termination_callback_;
OperationQueue operation_queue_;
