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

Issue 714280 link

Starred by 1 user
Status: Fixed
Owner:
tapted@chromium.org
Closed: May 2017
Cc:
█ mgiuca@chromium.org
tapted@chromium.org
gab@chromium.org
benwells@chromium.org
mac-bugs-priority@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

SequenceChecker failure in web_app::internals::CreatePlatformShortcuts

Project Member Reported by robertshield@chromium.org, Apr 21 2017 
Hello! 

I hit a failure running a recent trunk debug build of Mac Chrome while opening chrome with a user data dir flag set:

out/Debug/Google\ Chrome.app/Contents/MacOS/Google\ Chrome --user-data-dir=/Users/robertshield/tmp

I'm not an expert on sequence checker failures but there might be something wonky with the post task around here: https://codesearch.chromium.org/chromium/src/chrome/browser/web_applications/web_app.cc?l=127

[46373:35843:0421/164114.832639:FATAL:resource_bundle_mac.mm(93)] Check failed: sequence_checker_.CalledOnValidSequence().
0   libbase.dylib                       0x00000001111bc65e base::debug::StackTrace::StackTrace(unsigned long) + 174
1   libbase.dylib                       0x00000001111bc6fd base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x00000001111bab5c base::debug::StackTrace::StackTrace() + 28
3   libbase.dylib                       0x0000000111259aaf logging::LogMessage::~LogMessage() + 479
4   libbase.dylib                       0x0000000111257365 logging::LogMessage::~LogMessage() + 21
5   libui_base.dylib                    0x00000001167eb905 ui::ResourceBundle::GetNativeImageNamed(int) + 261
6   libchrome_dll.dylib                 0x000000011d771862 (anonymous namespace)::ImageRepForResource(int) + 50
7   libchrome_dll.dylib                 0x000000011d7683fb (anonymous namespace)::UpdateAppShortcutsSubdirLocalizedName(base::FilePath const&) + 1083
8   libchrome_dll.dylib                 0x000000011d7674d8 web_app::WebAppShortcutCreator::CreateShortcuts(web_app::ShortcutCreationReason, web_app::ShortcutLocations) + 696
9   libchrome_dll.dylib                 0x000000011d76f221 web_app::internals::CreatePlatformShortcuts(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason) + 609
10  libchrome_dll.dylib                 0x000000011d75cf50 bool base::internal::FunctorTraits<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason), void>::Invoke<base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason const&>(bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason), base::FilePath const&&&, web_app::ShortcutInfo const&&&, web_app::ShortcutLocations const&&&, web_app::ShortcutCreationReason const&&&) + 96
11  libchrome_dll.dylib                 0x000000011d75cee0 void base::internal::FunctorTraits<base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)>, void>::Invoke<base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)> const&, base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason const&>(base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)> const&&&, base::FilePath const&&&, web_app::ShortcutInfo const&&&, web_app::ShortcutLocations const&&&, web_app::ShortcutCreationReason const&&&) + 96
12  libchrome_dll.dylib                 0x000000011d75cddd void base::internal::InvokeHelper<false, void>::MakeItSo<base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)> const&, base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason const&>(base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)> const&&&, base::FilePath const&&&, web_app::ShortcutInfo const&&&, web_app::ShortcutLocations const&&&, web_app::ShortcutCreationReason const&&&) + 93
13  libchrome_dll.dylib                 0x000000011d75cd72 void base::internal::Invoker<base::internal::BindState<base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)>, base::FilePath, base::internal::ConstRefWrapper<web_app::ShortcutInfo>, web_app::ShortcutLocations, web_app::ShortcutCreationReason>, void ()>::RunImpl<base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)> const&, std::__1::tuple<base::FilePath, base::internal::ConstRefWrapper<web_app::ShortcutInfo>, web_app::ShortcutLocations, web_app::ShortcutCreationReason> const&, 0ul, 1ul, 2ul, 3ul>(base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)> const&&&, std::__1::tuple<base::FilePath, base::internal::ConstRefWrapper<web_app::ShortcutInfo>, web_app::ShortcutLocations, web_app::ShortcutCreationReason> const&&&, base::IndexSequence<0ul, 1ul, 2ul, 3ul>) + 242
14  libchrome_dll.dylib                 0x000000011d75cbec base::internal::Invoker<base::internal::BindState<base::internal::IgnoreResultHelper<bool (*)(base::FilePath const&, web_app::ShortcutInfo const&, web_app::ShortcutLocations const&, web_app::ShortcutCreationReason)>, base::FilePath, base::internal::ConstRefWrapper<web_app::ShortcutInfo>, web_app::ShortcutLocations, web_app::ShortcutCreationReason>, void ()>::Run(base::internal::BindStateBase*) + 44
15  libbase.dylib                       0x00000001111bee8f base::Callback<void (), (base::internal::CopyMode)0, (base::internal::RepeatMode)0>::Run() + 95
16  libbase.dylib                       0x0000000111466336 base::(anonymous namespace)::PostTaskAndReplyRelay::RunTaskAndPostReply() + 54
17  libbase.dylib                       0x0000000111466a95 void base::internal::FunctorTraits<void (base::(anonymous namespace)::PostTaskAndReplyRelay::*)(), void>::Invoke<base::(anonymous namespace)::PostTaskAndReplyRelay*>(void (base::(anonymous namespace)::PostTaskAndReplyRelay::*)(), base::(anonymous namespace)::PostTaskAndReplyRelay*&&) + 133
18  libbase.dylib                       0x00000001114669d4 void base::internal::InvokeHelper<false, void>::MakeItSo<void (base::(anonymous namespace)::PostTaskAndReplyRelay::*)(), base::(anonymous namespace)::PostTaskAndReplyRelay*>(void (base::(anonymous namespace)::PostTaskAndReplyRelay::*&&)(), base::(anonymous namespace)::PostTaskAndReplyRelay*&&) + 68
19  libbase.dylib                       0x0000000111466963 void base::internal::Invoker<base::internal::BindState<void (base::(anonymous namespace)::PostTaskAndReplyRelay::*)(), base::internal::UnretainedWrapper<base::(anonymous namespace)::PostTaskAndReplyRelay> >, void ()>::RunImpl<void (base::(anonymous namespace)::PostTaskAndReplyRelay::*)(), std::__1::tuple<base::internal::UnretainedWrapper<base::(anonymous namespace)::PostTaskAndReplyRelay> >, 0ul>(void (base::(anonymous namespace)::PostTaskAndReplyRelay::*&&)(), std::__1::tuple<base::internal::UnretainedWrapper<base::(anonymous namespace)::PostTaskAndReplyRelay> >&&, base::IndexSequence<0ul>) + 99
20  libbase.dylib                       0x00000001114668a9 base::internal::Invoker<base::internal::BindState<void (base::(anonymous namespace)::PostTaskAndReplyRelay::*)(), base::internal::UnretainedWrapper<base::(anonymous namespace)::PostTaskAndReplyRelay> >, void ()>::RunOnce(base::internal::BindStateBase*) + 57
21  libbase.dylib                       0x00000001111bee8f base::Callback<void (), (base::internal::CopyMode)0, (base::internal::RepeatMode)0>::Run() + 95
22  libbase.dylib                       0x00000001111bebd0 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 1024
23  libbase.dylib                       0x00000001112b1e3e base::MessageLoop::RunTask(base::PendingTask*) + 894
24  libbase.dylib                       0x00000001112b2394 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) + 68
25  libbase.dylib                       0x00000001112b2ddd base::MessageLoop::DoWork() + 669
26  libbase.dylib                       0x00000001112c334f base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) + 319
27  libbase.dylib                       0x00000001112b18c2 base::MessageLoop::RunHandler() + 610
28  libbase.dylib                       0x00000001113907cb base::RunLoop::Run() + 267
29  libbase.dylib                       0x000000011148b795 base::Thread::Run(base::RunLoop*) + 405
30  libcontent.dylib                    0x000000012d35d667 content::BrowserThreadImpl::FileThreadRun(base::RunLoop*) + 71
31  libcontent.dylib                    0x000000012d35dd0c content::BrowserThreadImpl::Run(base::RunLoop*) + 492
32  libbase.dylib                       0x000000011148c5f7 base::Thread::ThreadMain() + 2535
33  libbase.dylib                       0x0000000111465d61 base::(anonymous namespace)::ThreadFunc(void*) + 705
34  libsystem_pthread.dylib             0x00007fffb4f4baab _pthread_body + 180
35  libsystem_pthread.dylib             0x00007fffb4f4b9f7 _pthread_body + 0
36  libsystem_pthread.dylib             0x00007fffb4f4b1fd thread_start + 13

Comment 1 by tapted@chromium.org, Apr 22 2017

Cc: mgiuca@chromium.org
Owner: tapted@chromium.org
Status: Assigned (was: Available)
Thanks for reporting this!

Here, I think web_app_mac's UpdateAppShortcutsSubdirLocalizedName() [1] shouldn't be accessing ResourceBundle from the FILE thread, which it does in ImageRepForResource. So we need to pass in those (mac-specific) gfx::Image it might need. But the gfx::Image need to be destroyed on the UI thread still. Putting those gfx::Image on the web_app::ShortcutInfo is probably the neatest way to do that, since the crazy PostTask you point to is the mechanism used to ensure this happens for other images already.

[1] https://codesearch.chromium.org/chromium/src/chrome/browser/web_applications/web_app_mac.mm?type=cs&q=UpdateAppShortcutsSubdirLocalizedName&l=390

Comment 2 by gab@chromium.org, Apr 26 2017 

Thanks Robert for running a dcheck_always_on = true builds (everyone should!)

Indeed ResourceBundle shouldn't be accessed off the UI thread.

@tapted: if you're so inclined, we're currently migrating code to TaskScheduler and moving things off the FILE thread is one of the TODOs ( issue 653916 ): it'd be great if as part of touching this you could move this code to using a TaskScheduler provided SequencedTaskRunner (base/task_scheduler/post_task.h) instead of the FILE thread for things that need to be mutually exclusive in their world but that are otherwise independent from the rest of the work on the FILE thread.

Thanks,
Gab

Comment 3 by gab@chromium.org, Apr 26 2017

Labels: -Pri-3 Pri-1
Making P1, data races result in subtle bugs and are important to address quickly.
Project Member

Comment 4 by bugdroid1@chromium.org, May 2 2017 

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

commit 08fb0ca7077abf9c7ddfce597a4015244b16d434
Author: tzik <tzik@chromium.org>
Date: Tue May 02 18:40:26 2017

Do not touch ui::ResourceBundle on non-UI thread in web_app_mac.mm

gfx::ImageStorage has a non-thread-safe ref count, and gfx::Image in
ui::ResourceBundle needs to be copied or destroyed on the UI thread.

This CL removes an access to gfx::Image from non-UI thread in web_app_mac.mm,
to avoid a data race.

BUG= 714209 ,  714280 

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

[modify] https://crrev.com/08fb0ca7077abf9c7ddfce597a4015244b16d434/chrome/browser/web_applications/web_app_mac.mm

Comment 5 by tapted@chromium.org, May 26 2017

Status: Fixed (was: Assigned)
Filed  Issue 726573  for moving c/b/web_applications off the FILE thread. The sequencechecker stuff here is resolved by r468712
►

Sign in to add a comment