New issue
Advanced search Search tips

Issue 810139 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug
M-X



Sign in to add a comment

"UserManagerMacTest.ShowUserManager" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, Feb 7 2018

Issue description

"UserManagerMacTest.ShowUserManager" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyLQsSBUZsYWtlIiJVc2VyTWFuYWdlck1hY1Rlc3QuU2hvd1VzZXJNYW5hZ2VyDA.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 

Comment 1 by olka@chromium.org, Feb 8 2018

Cc: a...@chromium.org
Components: UI>Browser

Comment 2 by olka@chromium.org, Feb 9 2018

https://ci.chromium.org/buildbot/tryserver.chromium.mac/mac_chromium_rel_ng/645537:
[ RUN      ] UserManagerMacTest.ShowUserManager
[1377:775:0207/084552.703165:868542514637:FATAL:render_process_host_impl.cc(781)] Check failed: map_.empty().
0   unit_tests                          0x000000010638399c base::debug::StackTrace::StackTrace(unsigned long) + 28
1   unit_tests                          0x00000001063a8210 logging::LogMessage::~LogMessage() + 224
2   unit_tests                          0x0000000103a37228 content::(anonymous namespace)::SiteProcessCountTracker::~SiteProcessCountTracker() + 120
3   unit_tests                          0x0000000106420370 std::__1::__tree<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, void*>*) + 64
4   unit_tests                          0x000000010642034d std::__1::__tree<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, void*>*) + 29
5   unit_tests                          0x0000000106420359 std::__1::__tree<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, void*>*) + 41
6   unit_tests                          0x0000000106420359 std::__1::__tree<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, void*>*) + 41
7   unit_tests                          0x00000001064202d6 base::SupportsUserData::~SupportsUserData() + 198
8   unit_tests                          0x000000010362f951 content::BrowserContext::~BrowserContext() + 305
9   unit_tests                          0x000000010545182a TestingProfile::~TestingProfile() + 650
10  unit_tests                          0x000000010545191e TestingProfile::~TestingProfile() + 14
11  unit_tests                          0x0000000106934b46 ProfileDestroyer::DestroyProfileWhenAppropriate(Profile*) + 502
12  unit_tests                          0x0000000106958b50 std::__1::__tree<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::__map_value_compare<base::FilePath, std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::less<base::FilePath>, true>, std::__1::allocator<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, void*>*) + 80
13  unit_tests                          0x0000000106958b30 std::__1::__tree<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::__map_value_compare<base::FilePath, std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::less<base::FilePath>, true>, std::__1::allocator<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, void*>*) + 48
14  unit_tests                          0x000000010694f2a9 ProfileManager::~ProfileManager() + 233
15  unit_tests                          0x000000010545511e testing::ProfileManager::~ProfileManager() + 14
16  unit_tests                          0x0000000105453e45 TestingProfileManager::~TestingProfileManager() + 21
17  unit_tests                          0x00000001016c3b61 BrowserWithTestWindowTest::TearDown() + 289
18  unit_tests                          0x000000010264b8bc UserManagerMacTest::TearDown() + 76
19  unit_tests                          0x000000010288a880 testing::TestInfo::Run() + 288
20  unit_tests                          0x000000010288aea7 testing::TestCase::Run() + 263
21  unit_tests                          0x00000001028934a7 testing::internal::UnitTestImpl::RunAllTests() + 903
22  unit_tests                          0x00000001028930f3 testing::UnitTest::Run() + 163
23  unit_tests                          0x0000000105464827 base::TestSuite::Run() + 167
24  unit_tests                          0x00000001054749f8 base::(anonymous namespace)::LaunchUnitTestsInternal(base::RepeatingCallback<int ()> const&, unsigned long, int, bool, base::RepeatingCallback<void ()> const&) + 360
25  unit_tests                          0x000000010547486b base::LaunchUnitTests(int, char**, base::RepeatingCallback<int ()> const&) + 139
26  unit_tests                          0x0000000105455e19 main + 281

Comment 3 by olka@chromium.org, Feb 9 2018

Each time it flakes there is the same set of tests failing:
ColorPanelCocoaTest.ClearTargetAndDelegateOnEnd
UserManagerMacTest.ShowUserManager
SadTabViewBehaviorTest.ClickOnLinks
ColorPanelCocoaTest.ClearTargetOnEnd

Owner: ellyjo...@chromium.org
Status: Started (was: Untriaged)
All those tests are known broken on 10.13 - I've been fixing them today. I suspect the flake is just when a 10.13 bot happens to get selected from the pool.

Fixes:
ColorPanelCocoaTest: <https://chromium-review.googlesource.com/c/chromium/src/+/911191>
SadTabViewBehaviorTest: <https://chromium-review.googlesource.com/c/chromium/src/+/911708>

I'm working on UserManagerMacTest right now - it seems to be a UAF, possibly exposing a bug in how BrowserWithTestWindowTest::TearDown() works.

After the test, a BrowserContext object is destroyed:

[55046:775:0209/110626.094837:28183588156157:ERROR:browser_context.cc(570)] ~BrowserContext 0x7fdc6cf1c820
0   libbase.dylib                       0x000000011800ea8c base::debug::StackTrace::StackTrace(unsigned long) + 28
1   libcontent.dylib                    0x00000001192bb167 content::BrowserContext::~BrowserContext() + 135
2   unit_tests                          0x000000010e2ef4b3 TestingProfile::~TestingProfile() + 659
3   unit_tests                          0x000000010e2ef4ee TestingProfile::~TestingProfile() + 14
4   unit_tests                          0x000000010e7d5aa5 ProfileDestroyer::DestroyProfileWhenAppropriate(Profile*) + 261
5   unit_tests                          0x000000010e7f3a30 std::__1::__tree<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::__map_value_compare<base::FilePath, std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::less<base::FilePath>, true>, std::__1::allocator<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, void*>*) + 80
6   unit_tests                          0x000000010e7f3a10 std::__1::__tree<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::__map_value_compare<base::FilePath, std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::less<base::FilePath>, true>, std::__1::allocator<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, void*>*) + 48
7   unit_tests                          0x000000010e7ebc98 ProfileManager::~ProfileManager() + 120
8   unit_tests                          0x000000010e2f1aee testing::ProfileManager::~ProfileManager() + 14
9   unit_tests                          0x000000010e2f0fb5 TestingProfileManager::~TestingProfileManager() + 21
10  unit_tests                          0x000000010cdde5d1 BrowserWithTestWindowTest::TearDown() + 257
11  unit_tests                          0x000000010dccad65 UserManagerMacTest::TearDown() + 53
12  unit_tests                          0x000000010dec3f00 testing::TestInfo::Run() + 288
13  unit_tests                          0x000000010dec4467 testing::TestCase::Run() + 263
14  unit_tests                          0x000000010decb947 testing::internal::UnitTestImpl::RunAllTests() + 903
15  unit_tests                          0x000000010decb593 testing::UnitTest::Run() + 163
16  unit_tests                          0x000000010e2fb9c7 base::TestSuite::Run() + 167
17  unit_tests                          0x000000010e30918b base::LaunchUnitTests(int, char**, base::RepeatingCallback<int ()> const&) + 139
18  unit_tests                          0x000000010e2f22db main + 267
19  libdyld.dylib                       0x00007fff62f59115 start + 1

*Later*, that same object is accessed:

[55046:775:0209/110626.179501:28183672825711:ERROR:render_process_host_impl.cc(3232)]  BrowserContext 0x7fdc6cf1c820
0   libbase.dylib                       0x000000011800ea8c base::debug::StackTrace::StackTrace(unsigned long) + 28
1   libcontent.dylib                    0x00000001195b0525 content::RenderProcessHostImpl::UnregisterHost(int) + 501
2   unit_tests                          0x000000010e401df8 content::MockRenderProcessHost::~MockRenderProcessHost() + 632
3   unit_tests                          0x000000010e401fbe content::MockRenderProcessHost::~MockRenderProcessHost() + 14
4   unit_tests                          0x000000010e40375b content::MockRenderProcessHostFactory::~MockRenderProcessHostFactory() + 299
5   unit_tests                          0x000000010e40c9b7 content::RenderViewHostTestEnabler::~RenderViewHostTestEnabler() + 119
6   unit_tests                          0x000000010cdde197 BrowserWithTestWindowTest::~BrowserWithTestWindowTest() + 71
7   unit_tests                          0x000000010dccacfe UserManagerMacTest_ShowUserManager_Test::~UserManagerMacTest_ShowUserManager_Test() + 14
8   unit_tests                          0x000000010dec3f68 testing::TestInfo::Run() + 392
9   unit_tests                          0x000000010dec4467 testing::TestCase::Run() + 263
10  unit_tests                          0x000000010decb947 testing::internal::UnitTestImpl::RunAllTests() + 903
11  unit_tests                          0x000000010decb593 testing::UnitTest::Run() + 163
12  unit_tests                          0x000000010e2fb9c7 base::TestSuite::Run() + 167
13  unit_tests                          0x000000010e30918b base::LaunchUnitTests(int, char**, base::RepeatingCallback<int ()> const&) + 139
14  unit_tests                          0x000000010e2f22db main + 267
15  libdyld.dylib                       0x00007fff62f59115 start + 1

I'm working on figuring out why exactly this is now.
Cc: ellyjo...@chromium.org
Owner: est...@chromium.org
We're gonna need an expert here.

As far as I can tell, a MockRenderProcessHost is holding a reference (as a BrowserContext*) to a Profile which is actually owned by a ProfileManager, and the ProfileManager is destroyed (along with all the profiles) in BrowserWithTestWindowTest::TearDown, but the MockRenderProcessHost both still holds a reference and still tries to use it.

It seems like the Profile must outlive the MockRenderProcessHost (which is owned by a RenderViewHostTestEnabler, which is a member of BrowserWithTestWindowTest). However, if we simply don't delete the Profile or ProfileManager in BrowserWithTestWindowTest::TearDown(), we crash differently during ~BrowserWithTestWindowTest():

  * frame #0: 0x000000010266361d unit_tests`ProfileAttributesStorage::RemoveObserver(ProfileInfoCacheObserver*) [inlined] std::__1::vector<ProfileInfoCacheObserver*, std::__1::allocator<ProfileInfoCacheObserver*> >::begin(this=<unavailable>) at vector:1471
    frame #1: 0x000000010266361d unit_tests`ProfileAttributesStorage::RemoveObserver(ProfileInfoCacheObserver*) [inlined] base::ObserverList<ProfileInfoCacheObserver, false>::RemoveObserver(this=<unavailable>, obs=0x00000001180782a0) at observer_list.h:246
    frame #2: 0x000000010266361d unit_tests`ProfileAttributesStorage::RemoveObserver(this=0x0000000000000014, obs=0x00000001180782a0) + 13 at profile_attributes_storage.cc:333
    frame #3: 0x00000001034ccf17 unit_tests`UserManagerScreenHandler::ProfileUpdateObserver::~ProfileUpdateObserver() [inlined] UserManagerScreenHandler::ProfileUpdateObserver::~ProfileUpdateObserver(this=0x00000001180782a0) + 39 at user_manager_screen_handler.cc:248

[ extremely horrible stack trace elided ]

    frame #22: 0x000000010d580609 libcontent.dylib`content::WebUIImpl::~WebUIImpl(this=0x0000000118073dd0) + 9 at web_ui_impl.cc:89
    frame #23: 0x000000010d2b0aa2 libcontent.dylib`content::RenderFrameHostManager::ClearWebUIInstances(this=0x0000000120f2c520) + 18 at render_frame_host_manager.cc:460
    frame #24: 0x000000010d54f44b libcontent.dylib`content::WebContentsImpl::~WebContentsImpl(this=0x0000000119038200) + 763 at web_contents_impl.cc:615

So, there's clearly some ownership issues going on in this code and I'm not sure how to resolve them.

estade@, can you take a look? you touched this code recently, you might have more context :)
Project Member

Comment 6 by chromium...@appspot.gserviceaccount.com, Feb 9 2018

Detected 6 new flakes for test/step "UserManagerMacTest.ShowUserManager". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyLQsSBUZsYWtlIiJVc2VyTWFuYWdlck1hY1Rlc3QuU2hvd1VzZXJNYW5hZ2VyDA. This message was posted automatically by the chromium-try-flakes app.

Comment 7 by treib@chromium.org, Feb 12 2018

Labels: OS-Mac
Documenting the current state:
Re #3, it seems that everything except UserManagerMacTest.ShowUserManager has been fixed.
Re #4, I don't think this is related to 10.13 - as far as I can tell, all the failing bots were running macOS 10.12.2?

There haven't been any flakes over the last ~2.5 days, which is presumably because nobody was running tryjobs over the weekend :)
Labels: -Sheriff-Chromium
Project Member

Comment 9 by chromium...@appspot.gserviceaccount.com, Feb 13 2018

Labels: Sheriff-Chromium
Detected 4 new flakes for test/step "UserManagerMacTest.ShowUserManager". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyLQsSBUZsYWtlIiJVc2VyTWFuYWdlck1hY1Rlc3QuU2hvd1VzZXJNYW5hZ2VyDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
Owner: ellyjo...@chromium.org
Status: Assigned (was: Started)
what code did I touch recently? I'm very unlikely to be able to debug a mac specific failure without actually having a mac.
Labels: -Sheriff-Chromium
Project Member

Comment 12 by chromium...@appspot.gserviceaccount.com, Feb 14 2018

Labels: Sheriff-Chromium
Detected 16 new flakes for test/step "UserManagerMacTest.ShowUserManager". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyLQsSBUZsYWtlIiJVc2VyTWFuYWdlck1hY1Rlc3QuU2hvd1VzZXJNYW5hZ2VyDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
Status: Started (was: Assigned)
I'm disabling this test for now: <https://chromium-review.googlesource.com/c/chromium/src/+/919463> pending investigation of the problem described in #5.
Project Member

Comment 14 by bugdroid1@chromium.org, Feb 14 2018

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

commit b5aaf37eddfa619e5607d6b4193af7c42a965aed
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Feb 14 18:15:51 2018

cocoa: disable UserManagerMacTest

There's a subtle lifetime issue exposed by this test. See the linked bug for
more details.

Bug: 810139
Change-Id: If04cc1573c7983ab62627770ea96c7a5ef6a56cb
Reviewed-on: https://chromium-review.googlesource.com/919463
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536756}
[modify] https://crrev.com/b5aaf37eddfa619e5607d6b4193af7c42a965aed/chrome/browser/ui/cocoa/profiles/user_manager_mac_unittest.mm

Labels: -Pri-1 -Sheriff-Chromium Pri-2
Untagging sheriffs and dropping priority, since this test is now disabled.
Labels: Hotlist-DesktopUIToolingRequired Hotlist-DesktopUIChecked
***UI Mass Triage ***
Labels: -Pri-2 M-X Pri-3
Owner: ----
Status: Available (was: Started)

Sign in to add a comment