Commit 280a3b3f authored by Ahmed Fakhry's avatar Ahmed Fakhry Committed by Commit Bot

Task Manager: Speculative crash fix

It's not clear to me why calling base::ThreadTaskRunnerHandle::Get()
from ArcProcessTaskProvider::ScheduleNextRequest() would fail,
especially since this is happening on the UI thread. But I added a few
things that may fix the issue:
- I noticed that ArcProcessService dtor may be called, but we did't
  stop observing the ProcessSnapshotServer.
- When the task manager is closed, let's invalidate all the weak_ptrs of
  ArcProcessTaskProvider, so we don't get any pending requested from
  ArcProcessService after that.

BUG=1136378

Change-Id: Icdbc8161e397899a2decd7efac13a39462bb6d71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2536812Reviewed-by: default avatarWillie Koomson <wvk@google.com>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827898}
parent 2b495cd4
......@@ -306,6 +306,8 @@ ArcProcessService::ArcProcessService(content::BrowserContext* context,
ArcProcessService::~ArcProcessService() {
arc_bridge_service_->process()->RemoveObserver(this);
if (is_observing_process_snapshot_)
ProcessSnapshotServer::Get()->RemoveObserver(this);
}
// static
......@@ -327,6 +329,7 @@ ArcProcessService* ArcProcessService::Get() {
void ArcProcessService::RequestAppProcessList(
RequestProcessListCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
HandleRequest(
base::BindOnce(&ArcProcessService::ContinueAppProcessListRequest,
base::Unretained(this), std::move(callback)));
......@@ -334,6 +337,7 @@ void ArcProcessService::RequestAppProcessList(
void ArcProcessService::RequestSystemProcessList(
RequestProcessListCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
HandleRequest(
base::BindOnce(&ArcProcessService::ContinueSystemProcessListRequest,
base::Unretained(this), std::move(callback)));
......@@ -341,12 +345,14 @@ void ArcProcessService::RequestSystemProcessList(
void ArcProcessService::RequestAppMemoryInfo(
RequestMemoryInfoCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
HandleRequest(base::BindOnce(&ArcProcessService::ContinueAppMemoryInfoRequest,
base::Unretained(this), std::move(callback)));
}
void ArcProcessService::RequestSystemMemoryInfo(
RequestMemoryInfoCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
HandleRequest(
base::BindOnce(&ArcProcessService::ContinueSystemMemoryInfoRequest,
base::Unretained(this), std::move(callback)));
......@@ -354,6 +360,7 @@ void ArcProcessService::RequestSystemMemoryInfo(
void ArcProcessService::OnProcessSnapshotRefreshed(
const base::ProcessIterator::ProcessEntries& snapshot) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
cached_process_snapshot_ = snapshot;
last_process_snapshot_time_ = base::Time::Now();
......@@ -464,6 +471,7 @@ void ArcProcessService::HandleRequest(base::OnceClosure request) {
void ArcProcessService::ContinueAppProcessListRequest(
RequestProcessListCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// Since several services call this class to get information about the ARC
// process list, it can produce a lot of logspam when the board is ARC-ready
// but the user has not opted into ARC. This redundant check avoids that
......@@ -487,6 +495,7 @@ void ArcProcessService::ContinueAppProcessListRequest(
void ArcProcessService::ContinueSystemProcessListRequest(
RequestProcessListCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::PostTaskAndReplyWithResult(
task_runner_.get(), FROM_HERE,
base::BindOnce(&GetArcSystemProcessList, cached_process_snapshot_),
......@@ -497,6 +506,7 @@ void ArcProcessService::ContinueSystemProcessListRequest(
void ArcProcessService::ContinueAppMemoryInfoRequest(
RequestMemoryInfoCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!connection_ready_) {
std::move(callback).Run({});
return;
......@@ -518,6 +528,7 @@ void ArcProcessService::ContinueAppMemoryInfoRequest(
void ArcProcessService::ContinueSystemMemoryInfoRequest(
RequestMemoryInfoCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!connection_ready_) {
std::move(callback).Run({});
return;
......
......@@ -134,6 +134,7 @@ void ArcProcessTaskProvider::StartUpdating() {
void ArcProcessTaskProvider::StopUpdating() {
is_updating_ = false;
weak_ptr_factory_.InvalidateWeakPtrs();
nspid_to_task_.clear();
nspid_to_sys_task_.clear();
}
......
......@@ -51,10 +51,6 @@ class VmProcessTaskProvider : public TaskProvider,
// Map of PIDs to the corresponding Task object for a running VM.
base::flat_map<base::ProcessId, std::unique_ptr<VmProcessTask>> task_map_;
// Always keep this the last member of this class to make sure it's the
// first thing to be destructed.
base::WeakPtrFactory<VmProcessTaskProvider> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(VmProcessTaskProvider);
};
......
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