New issue
Advanced search Search tips

Issue 829786 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Shutdown hang when single-threaded BLOCK_SHUTDOWN task is posted after single-threaded CONTINUE_ON_SHUTDOWN task

Project Member Reported by grt@chromium.org, Apr 6 2018

Issue description

I have a dump from a shutdown hang (67.0.3387.0 canary) that appears to be due to a task blocking shutdown, but I don't see such a task.

The UI thread is blocked on the Wait in TaskTracker::PerformShutdown:

  // It is safe to access |shutdown_event_| without holding |lock_| because the
  // pointer never changes after being set above.
  {
    base::ThreadRestrictions::ScopedAllowWait allow_wait;
    shutdown_event_->Wait();
  }

The only thread that's within a call to RunTask is on a COM thread that was supposedly started with CONTINUE_ON_SHUTDOWN. It's posted from here:

void BackgroundDownloader::DoStartDownload(const GURL& url) {
  DCHECK(thread_checker_.CalledOnValidThread());
  com_task_runner_->PostTask(
      FROM_HERE, base::BindOnce(&BackgroundDownloader::BeginDownload,
                                base::Unretained(this), url));
}

and the task runner was created with:

      com_task_runner_(base::CreateCOMSTATaskRunnerWithTraits(
          kTaskTraitsBackgroundDownloader)),

where kTaskTraitsBackgroundDownloader is:

constexpr base::TaskTraits kTaskTraitsBackgroundDownloader = {
    base::MayBlock(), base::TaskPriority::BACKGROUND,
    base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN};

Here's a full memory dump if anyone wants to take a look: https://drive.google.com/file/d/0B54adY1TO21BM0VHVTBQZFhEU1k/view?usp=sharing.
 
Owner: fdoray@chromium.org
Status: Assigned (was: Untriaged)
By default, SingleThreadTaskRunners obtained from base::CreateCOMSTATaskRunnerWithTraits() with the same TaskPriority and MayBlock traits share the same thread [1]. Unfortunately, that means that if a COM/BACKGROUND/MayBlock/BLOCK_SHUTDOWN task_A is posted after a COM/BACKGROUND/MayBlock/CONTINUE_ON_SHUTDOWN task_B, shutdown will be blocked on both task_B and task_A.

My solution to this problem would be to send single-threaded CONTINUE_ON_SHUTDOWN and non-CONTINUE_ON_SHUTDOWN tasks to separate threads.

[1] https://cs.chromium.org/chromium/src/base/task_scheduler/single_thread_task_runner_thread_mode.h?l=10&rcl=7a4a494218de1fceed5ff9ce5c51d946cef7339d
Summary: Shutdown hang when single-threaded BLOCK_SHUTDOWN task is posted after single-threaded CONTINUE_ON_SHUTDOWN task (was: Shutdown hang waiting on I don't know what.)

Comment 3 by gab@chromium.org, Apr 6 2018

Agreed with solution in #1, thanks!
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a016f19dced52530584d5e8609bca4b1ce9a0453

commit a016f19dced52530584d5e8609bca4b1ce9a0453
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Apr 12 15:44:18 2018

TaskScheduler: Prevent single-threaded CONTINUE_ON_SHUTDOWN from blocking shutdown.

Previously, tasks posted to a shared SingleThreadTaskRunner with the
same traits were scheduled on the same thread. If single-threaded task A
(BACKGROUND, CONTINUE_ON_SHUTDOWN) was scheduled on the thread and
single-threaded task B (BACKGROUND, BLOCK_SHUTDOWN) was posted
afterwards, shutdown could not complete until task A finished and
allowed task B to run on the thread.

This CL fixes the issue by scheduling shared single-threaded tasks
that are CONTINUE_ON_SHUTDOWN/non-CONTINUE_ON_SHUTDOWN on different
threads.

Bug:  829786 
Change-Id: Ia0b556971895c2a5799157f8dcf1a1ee6ce650c9
Reviewed-on: https://chromium-review.googlesource.com/1005422
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550216}
[modify] https://crrev.com/a016f19dced52530584d5e8609bca4b1ce9a0453/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
[modify] https://crrev.com/a016f19dced52530584d5e8609bca4b1ce9a0453/base/task_scheduler/scheduler_single_thread_task_runner_manager.h
[modify] https://crrev.com/a016f19dced52530584d5e8609bca4b1ce9a0453/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc

Comment 5 by fdoray@chromium.org, Apr 12 2018

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a016f19dced52530584d5e8609bca4b1ce9a0453

commit a016f19dced52530584d5e8609bca4b1ce9a0453
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Apr 12 15:44:18 2018

TaskScheduler: Prevent single-threaded CONTINUE_ON_SHUTDOWN from blocking shutdown.

Previously, tasks posted to a shared SingleThreadTaskRunner with the
same traits were scheduled on the same thread. If single-threaded task A
(BACKGROUND, CONTINUE_ON_SHUTDOWN) was scheduled on the thread and
single-threaded task B (BACKGROUND, BLOCK_SHUTDOWN) was posted
afterwards, shutdown could not complete until task A finished and
allowed task B to run on the thread.

This CL fixes the issue by scheduling shared single-threaded tasks
that are CONTINUE_ON_SHUTDOWN/non-CONTINUE_ON_SHUTDOWN on different
threads.

Bug:  829786 
Change-Id: Ia0b556971895c2a5799157f8dcf1a1ee6ce650c9
Reviewed-on: https://chromium-review.googlesource.com/1005422
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550216}
[modify] https://crrev.com/a016f19dced52530584d5e8609bca4b1ce9a0453/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
[modify] https://crrev.com/a016f19dced52530584d5e8609bca4b1ce9a0453/base/task_scheduler/scheduler_single_thread_task_runner_manager.h
[modify] https://crrev.com/a016f19dced52530584d5e8609bca4b1ce9a0453/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc

Sign in to add a comment