New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 787259 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Hotlist-MemoryInfra

Blocking:
issue 706592
issue 738275



Sign in to add a comment

MemoryDumpManager handling of TaskRunner failure is racey.

Project Member Reported by w...@chromium.org, Nov 21 2017

Issue description

MemoryDumpManager includes logic to ensure that a memory dump doesn't hang if one or more providers' TaskRunners stop accepting tasks:

  bool did_post_task = task_runner->PostTask(
      FROM_HERE, BindOnce(&MemoryDumpManager::InvokeOnMemoryDump,
                          Unretained(this),
                          Unretained(pmd_async_state.get())));

  if (did_post_task) {
    // Ownership is tranferred to InvokeOnMemoryDump().
    ignore_result(pmd_async_state.release());
    return;
  }

This assumes that TaskRunner::PostTask() will return true if the task will be executed, and false otherwise. The API contract of PostTask(), however, is to return true if the task _may_ be executed, and false if it definitely will not.

If this fires during testing, it will manifest as MemoryDumpManagerTest.PostTaskForSequencedTaskRunner timing-out; the test should probably be updated to e.g. post a delayed task to quit the RunLoop::Run() some time before the test's timeout is due, to make the failure-mode easier to spot.

I believe this was the cause of a timeout flake under Fuchsia/x64 (see https://build.chromium.org/p/chromium.fyi/builders/Fuchsia/builds/11278).
 

Comment 1 by w...@chromium.org, Nov 21 2017

Cc: scottmg@chromium.org
Blocking: 738275 706592

Sign in to add a comment