New issue
Advanced search Search tips

Issue 675800 link

Starred by 3 users
Status: Fixed
Owner:
fdoray@chromium.org
Closed: Dec 2016
Cc:
robliao@chromium.org
gab@chromium.org
fdoray@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 661143



Sign in to add a comment

Crash in history::HistoryBackend::ProcessDBTask behind BrowserScheduler.RedirectHistoryService experiment.

Project Member Reported by robliao@chromium.org, Dec 20 2016 
Looks related to History Service redirection

Truncated thread stack for readability (Full stack attached):
0:021> kn20
 # ChildEBP RetAddr  
00 (Inline) -------- chrome_5a5f0000!scoped_refptr<base::SingleThreadTaskRunner>::{ctor}+0x80a244
01 2c80f36c 5d0d93b2 chrome_5a5f0000!base::ThreadTaskRunnerHandle::Get+0x48
02 2c80f44c 5d0da6bc chrome_5a5f0000!syncer::SharedChangeProcessor::Connect+0x1e
03 2c80f5cc 5d0d1e08 chrome_5a5f0000!syncer::SharedChangeProcessor::StartAssociation+0xa2
04 2c80f5ec 5d0d1e82 chrome_5a5f0000!base::internal::FunctorTraits<void (__thiscall syncer::Share...
05 2c80f614 5d0d1eba chrome_5a5f0000!base::internal::InvokeHelper<0,void>::MakeItSo<void (__thisc...
06 2c80f640 5d0d2c37 chrome_5a5f0000!base::internal::Invoker<base::internal::BindState<void (__th...
07 2c80f654 5d545128 chrome_5a5f0000!base::internal::Invoker<base::internal::BindState<void (__th...
08 (Inline) -------- chrome_5a5f0000!base::internal::RunMixin<base::Callback<void __cdecl(void),1...
09 2c80f680 5bb02f11 chrome_5a5f0000!browser_sync::`anonymous namespace'::RunTaskOnHistoryThread:...
0a 2c80f6a0 5c66a0e0 chrome_5a5f0000!std::list<std::unique_ptr<content::VideoCaptureController::C...
0b 2c80f6b0 5c650c23 chrome_5a5f0000!history::HistoryBackend::ProcessDBTask+0x4e
0c 2c80f6c8 5c651438 chrome_5a5f0000!base::internal::FunctorTraits<void (__thiscall history::Hist...
0d (Inline) -------- chrome_5a5f0000!base::internal::InvokeHelper<0,void>::MakeItSo+0x820613dd
0e 2c80f6f0 5c65534c chrome_5a5f0000!base::internal::Invoker<base::internal::BindState<void (__th...
0f 2c80f704 5b0b5d1b chrome_5a5f0000!base::internal::Invoker<base::internal::BindState<void (__th...
10 (Inline) -------- chrome_5a5f0000!base::internal::RunMixin<base::Callback<void __cdecl(void),0...
11 2c80f7cc 5b0c01b6 chrome_5a5f0000!base::debug::TaskAnnotator::RunTask+0x15d
12 2c80f7e4 5b0bffdd chrome_5a5f0000!base::internal::TaskTracker::PerformRunTask+0x15
13 2c80f860 5b0c245b chrome_5a5f0000!base::internal::TaskTracker::RunTask+0x29b
14 2c80fb78 5b00af29 chrome_5a5f0000!base::internal::SchedulerWorker::Thread::ThreadMain+0x265
15 2c80fb9c 764138f4 chrome_5a5f0000!base::`anonymous namespace'::ThreadFunc+0x8b

Comment 1 by robliao@chromium.org, Dec 20 2016

Stack.txt
10.7 KB View Download

Comment 2 by robliao@chromium.org, Dec 20 2016

Blocking: -661143 653916
Owner: gab@chromium.org

Comment 3 by robliao@chromium.org, Dec 20 2016

Description: Show this description

Comment 4 by robliao@chromium.org, Dec 20 2016

Blocking: -653916 661143
Owner: fdoray@chromium.org

Comment 5 by robliao@chromium.org, Dec 20 2016

Description: Show this description

Comment 6 by gab@chromium.org, Dec 20 2016

Labels: -Pri-2 Pri-1
Status: Started (was: Available)

Comment 7 by gab@chromium.org, Dec 20 2016

Summary: Crash in history::HistoryBackend::ProcessDBTask behind BrowserScheduler.RedirectHistoryService experiment. (was: Crash in history::HistoryBackend::ProcessDBTask)
Project Member

Comment 8 by sheriffbot@chromium.org, Dec 21 2016

Labels: OS-Windows FoundIn-M-57 Fracas OS-Mac
Users experienced this crash on the following builds:

Win Canary 57.0.2957.0 -  2.34 CPM, 9 reports, 6 clients (signature base::ThreadTaskRunnerHandle::Get)
Mac Canary 57.0.2957.0 -  14.10 CPM, 13 reports, 8 clients (signature base::ThreadTaskRunnerHandle::Get)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 9 by ajha@chromium.org, Dec 21 2016

Labels: -Type-Bug M-57 OS-Android Type-Bug-Regression
These crashes spiked from chrome version: 57.0.2954.0 and crashes are seen on the latest canary of Android(57.0.2957.0) as well.
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 21 2016 

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

commit 6fce596dd212d98fba4603f1806653809fb85c30
Author: fdoray <fdoray@chromium.org>
Date: Wed Dec 21 16:12:27 2016

Make TypedUrlSyncableService sequence-affine.

This class is thread-unsafe not thread-affine. It requires
external synchronization between tasks; which a sequence brings
implicitly. It does not need to be used from a single thread.

Note: Classes that do not depend on a constant thread id (implies that
they do not use thread-local storage) are usually not thread-affine.

BUG= 675800 ,  675631 

Review-Url: https://codereview.chromium.org/2591643004
Cr-Commit-Position: refs/heads/master@{#440115}

[modify] https://crrev.com/6fce596dd212d98fba4603f1806653809fb85c30/components/history/core/browser/typed_url_syncable_service.cc
[modify] https://crrev.com/6fce596dd212d98fba4603f1806653809fb85c30/components/history/core/browser/typed_url_syncable_service.h
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 21 2016 

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

commit ab2ab7ec3a72fcb6c023f0ccbf7971af1031805a
Author: fdoray <fdoray@chromium.org>
Date: Wed Dec 21 20:10:49 2016

Make sync's change processors sequence-affine.

These classes are thread-unsafe not thread-affine. They require
external synchronization between tasks; which a sequence brings them
implicitly. They do not need to be used from a single thread.

Note: Classes that do not depend on a constant thread id (implies that
they do not use thread-local storage) are usually not thread-affine.

BUG= 675800 ,  675631 

Review-Url: https://codereview.chromium.org/2593803002
Cr-Commit-Position: refs/heads/master@{#440195}

[modify] https://crrev.com/ab2ab7ec3a72fcb6c023f0ccbf7971af1031805a/components/sync/driver/generic_change_processor.cc
[modify] https://crrev.com/ab2ab7ec3a72fcb6c023f0ccbf7971af1031805a/components/sync/driver/generic_change_processor.h
[modify] https://crrev.com/ab2ab7ec3a72fcb6c023f0ccbf7971af1031805a/components/sync/driver/shared_change_processor.cc
[modify] https://crrev.com/ab2ab7ec3a72fcb6c023f0ccbf7971af1031805a/components/sync/driver/shared_change_processor.h
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 22 2016 

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

commit 0b0b5aab6e2474e0e758ca3777b07321e59a7463
Author: fdoray <fdoray@chromium.org>
Date: Thu Dec 22 14:09:12 2016

Document that HistoryBackend is sequence-affine, not thread-affine.

HistoryBackend methods must not be called concurrently and external
synchronization is required between calls on different thread.
However, the methods do not have to be called from a single thread.
This CL documents this and adds DCHECKs to verify that
HistoryBackend methods are called in sequence.

BUG= 675800 ,  675631 

Review-Url: https://codereview.chromium.org/2591123004
Cr-Commit-Position: refs/heads/master@{#440408}

[modify] https://crrev.com/0b0b5aab6e2474e0e758ca3777b07321e59a7463/components/browser_sync/profile_sync_service_typed_url_unittest.cc
[modify] https://crrev.com/0b0b5aab6e2474e0e758ca3777b07321e59a7463/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/0b0b5aab6e2474e0e758ca3777b07321e59a7463/components/history/core/browser/history_backend.h
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 22 2016 

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

commit cc870ed0d9a16f478dd5e2f584ed19b749f2729f
Author: penghuang <penghuang@chromium.org>
Date: Thu Dec 22 16:55:05 2016

Revert of Document that HistoryBackend is sequence-affine, not thread-affine. (patchset #2 id:20001 of https://codereview.chromium.org/2591123004/ )

Reason for revert:
https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/20846/steps/browser_tests/logs/ProfileBrowserTest.OffTheRecordURLRequestContextIsolation

The CL causes the browser_tests failure on Linux ChromiumOS Tests(dbg)

Original issue's description:
> Document that HistoryBackend is sequence-affine, not thread-affine.
>
> HistoryBackend methods must not be called concurrently and external
> synchronization is required between calls on different thread.
> However, the methods do not have to be called from a single thread.
> This CL documents this and adds DCHECKs to verify that
> HistoryBackend methods are called in sequence.
>
> BUG= 675800 ,  675631 
>
> Committed: https://crrev.com/0b0b5aab6e2474e0e758ca3777b07321e59a7463
> Cr-Commit-Position: refs/heads/master@{#440408}

TBR=sdefresne@chromium.org,fdoray@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 675800 ,  675631 

Review-Url: https://codereview.chromium.org/2599803002
Cr-Commit-Position: refs/heads/master@{#440441}

[modify] https://crrev.com/cc870ed0d9a16f478dd5e2f584ed19b749f2729f/components/browser_sync/profile_sync_service_typed_url_unittest.cc
[modify] https://crrev.com/cc870ed0d9a16f478dd5e2f584ed19b749f2729f/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/cc870ed0d9a16f478dd5e2f584ed19b749f2729f/components/history/core/browser/history_backend.h

Comment 14 by fdoray@chromium.org, Dec 23 2016

Status: Fixed (was: Started)
►

Sign in to add a comment