Commit 712d1cf9 authored by Hector Dearman's avatar Hector Dearman Committed by Commit Bot

memory-infra: Don't consider clients with null PIDs

After crrev.com/c/565510 the PID is received asynchronously
after the start of a service. It is possible to get a dump
request between the point when a client process is started
and the point when the PID is received. In this case just
temporarily skip that client's dumps.

Bug: 744722
Change-Id: I69a70747408232063a1d57933d8d0ff985e153c9
Reviewed-on: https://chromium-review.googlesource.com/575127
Commit-Queue: Hector Dearman <hjd@chromium.org>
Reviewed-by: default avatarPrimiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487487}
parent 815ad5ba
...@@ -238,6 +238,7 @@ void CoordinatorImpl::PerformNextQueuedGlobalMemoryDump() { ...@@ -238,6 +238,7 @@ void CoordinatorImpl::PerformNextQueuedGlobalMemoryDump() {
// TODO(hjd): Change to NOTREACHED when crbug.com/739710 is fixed. // TODO(hjd): Change to NOTREACHED when crbug.com/739710 is fixed.
VLOG(1) << "Couldn't find a PID for client \"" << client_identity.name() VLOG(1) << "Couldn't find a PID for client \"" << client_identity.name()
<< "." << client_identity.instance() << "\""; << "." << client_identity.instance() << "\"";
continue;
} }
request->responses[client].process_id = pid; request->responses[client].process_id = pid;
request->responses[client].process_type = kv.second->process_type; request->responses[client].process_type = kv.second->process_type;
...@@ -265,6 +266,12 @@ void CoordinatorImpl::PerformNextQueuedGlobalMemoryDump() { ...@@ -265,6 +266,12 @@ void CoordinatorImpl::PerformNextQueuedGlobalMemoryDump() {
for (const auto& kv : clients_) { for (const auto& kv : clients_) {
service_manager::Identity client_identity = kv.second->identity; service_manager::Identity client_identity = kv.second->identity;
const base::ProcessId pid = GetProcessIdForClientIdentity(client_identity); const base::ProcessId pid = GetProcessIdForClientIdentity(client_identity);
if (pid == base::kNullProcessId) {
// TODO(hjd): Change to NOTREACHED when crbug.com/739710 is fixed.
VLOG(1) << "Couldn't find a PID for client \"" << client_identity.name()
<< "." << client_identity.instance() << "\"";
continue;
}
pids.push_back(pid); pids.push_back(pid);
if (kv.second->process_type == mojom::ProcessType::BROWSER) { if (kv.second->process_type == mojom::ProcessType::BROWSER) {
browser_client = kv.first; browser_client = kv.first;
...@@ -416,10 +423,8 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() { ...@@ -416,10 +423,8 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
std::map<base::ProcessId, mojom::ProcessMemoryDumpPtr> finalized_pmds; std::map<base::ProcessId, mojom::ProcessMemoryDumpPtr> finalized_pmds;
for (auto& response : request->responses) { for (auto& response : request->responses) {
const base::ProcessId pid = response.second.process_id; const base::ProcessId pid = response.second.process_id;
// TODO(hjd): this shouldn't really happen, but seem to be hitting this in DCHECK(!finalized_pmds.count(pid))
// crbug.com/744722 . Temporarily softening the DCHECK while investigating. << "Received PID: " << pid << " more than once.";
if (finalized_pmds.count(pid) > 0)
continue;
// The dump might be nullptr if the client crashed / disconnected before // The dump might be nullptr if the client crashed / disconnected before
// replying. // replying.
......
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