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

Issue 779504 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: ----



Sign in to add a comment

unit_tests failing on multiple builders

Project Member Reported by tschumann@chromium.org, Oct 30 2017

Issue description

assigning to wzang who recently refactored the wallpaper management which seems related to the error message (https://chromium-review.googlesource.com/734801).

cc'ing skuhne who owns MultiUserWindowManagerChromeOSTest

unit_tests failing on multiple builders

Builders failed on: 
- Linux ChromiumOS Tests (dbg)(1): 
  https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29
- Linux Chromium OS ASan LSan Tests (1): 
  https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29

some examples:
https://chromium-swarm.appspot.com/task?id=39856429895e4010&refresh=10&show_raw=1
https://chromium-swarm.appspot.com/task?id=3985808dcb14f410&refresh=10&show_raw=1
https://chromium-swarm.appspot.com/task?id=3984b81ccddabe10&refresh=10&show_raw=1

Different test cases of MultiUserWindowManagerChromeOSTest are affected but fail with messages like this one:

[ RUN      ] MultiUserWindowManagerChromeOSTest.FindBrowserWithActiveWindow
[8471:8471:1030/025255.172589:15093340233:WARNING:install_attributes.cc(117)] Install attributes missing, first sign in
[8471:8471:1030/025255.445674:15093613318:FATAL:wallpaper_manager.cc(633)] Check failed: wallpaper_manager. 
#0 0x7f3964f4304d base::debug::StackTrace::StackTrace()
#1 0x7f3964f415cc base::debug::StackTrace::StackTrace()
#2 0x7f3964fc493d logging::LogMessage::~LogMessage()
#3 0x000005ba644a chromeos::WallpaperManager::Get()
#4 0x000005bbf3d9 chromeos::WallpaperManager::PendingWallpaper::ProcessRequest()
#5 0x0000020babff _ZN4base8internal13FunctorTraitsIMN10extensions12image_writer9OperationEFvvEvE6InvokeIRK13scoped_refptrINS3_26DestroyPartitionsOperationEEJEEEvS6_OT_DpOT0_
#6 0x000005bbff94 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN8chromeos16WallpaperManager16PendingWallpaperEFvvEJRK13scoped_refptrIS6_EEEEvOT_DpOT0_
#7 0x000005bbff40 _ZN4base8internal7InvokerINS0_9BindStateIMN8chromeos16WallpaperManager16PendingWallpaperEFvvEJ13scoped_refptrIS5_EEEEFvvEE7RunImplIRKS7_RKNSt3__15tupleIJS9_EEEJLm0EEEEvOT_OT0_NSG_16integer_sequenceImJXspT1_EEEE
#8 0x000005bbfe8c _ZN4base8internal7InvokerINS0_9BindStateIMN8chromeos16WallpaperManager16PendingWallpaperEFvvEJ13scoped_refptrIS5_EEEEFvvEE3RunEPNS0_13BindStateBaseE
#9 0x7f3964eebb0d _ZNKR4base17RepeatingCallbackIFvvEE3RunEv
#10 0x7f396516dba8 base::Timer::RunScheduledTask()
#11 0x7f396516dcc9 base::BaseTimerTaskInternal::Run()
#12 0x7f3964f5cb2d _ZN4base8internal13FunctorTraitsIMNS_21FileDescriptorWatcher10Controller7WatcherEFvvEvE6InvokeIPS4_JEEEvS6_OT_DpOT0_
#13 0x7f3964f5ca74 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMNS_21FileDescriptorWatcher10Controller7WatcherEFvvEJPS6_EEEvOT_DpOT0_
#14 0x7f396516de25 _ZN4base8internal7InvokerINS0_9BindStateIMNS_21BaseTimerTaskInternalEFvvEJNS0_12OwnedWrapperIS3_EEEEEFvvEE7RunImplIS5_NSt3__15tupleIJS7_EEEJLm0EEEEvOT_OT0_NSC_16integer_sequenceImJXspT1_EEEE
#15 0x7f396516dd79 _ZN4base8internal7InvokerINS0_9BindStateIMNS_21BaseTimerTaskInternalEFvvEJNS0_12OwnedWrapperIS3_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE
#16 0x7f3964ef0e91 _ZNO4base12OnceCallbackIFvvEE3RunEv
#17 0x7f3964f47990 base::debug::TaskAnnotator::RunTask()
#18 0x7f3964fe5cf3 base::internal::IncomingTaskQueue::RunTask()
#19 0x7f3964fedf6f base::MessageLoop::RunTask()
#20 0x7f3964fee1e3 base::MessageLoop::DeferOrRunPendingTask()
#21 0x7f3964fee7a2 base::MessageLoop::DoDelayedWork()
#22 0x7f3964ff458d base::MessagePumpLibevent::Run()
#23 0x7f3964fed836 base::MessageLoop::Run()
#24 0x7f396509c65d base::RunLoop::Run()
#25 0x7f396509d2b7 base::RunLoop::RunUntilIdle()
#26 0x000006a1924c content::TestBrowserThreadBundle::~TestBrowserThreadBundle()
#27 0x00000ce5a552 ash::AshTestEnvironmentContent::~AshTestEnvironmentContent()
#28 0x00000ce5a599 ash::AshTestEnvironmentContent::~AshTestEnvironmentContent()
#29 0x00000cc17aeb ash::AshTestBase::~AshTestBase()
#30 0x000001dcd869 ash::MultiUserWindowManagerChromeOSTest::~MultiUserWindowManagerChromeOSTest()
#31 0x000001dcd8d5 ash::MultiUserWindowManagerChromeOSTest_BasicTests_Test::~MultiUserWindowManagerChromeOSTest_BasicTests_Test()
#32 0x000001dcd8f9 ash::MultiUserWindowManagerChromeOSTest_BasicTests_Test::~MultiUserWindowManagerChromeOSTest_BasicTests_Test()
#33 0x000000994c9b base::RefCountedThreadSafe<>::DeleteInternal<>()
#34 0x0000028d0f7e _ZN7testing8internal12InvokeHelperIN16sync_file_system18RemoteServiceStateENSt3__15tupleIJEEEE12InvokeMethodINS2_25MockRemoteFileSyncServiceEMS9_KFS3_vEEES3_PT_T0_RKS6_
#35 0x000003ee4f22 testing::internal::HandleExceptionsInMethodIfSupported<>()
#36 0x000003ed0df5 testing::TestInfo::Run()
#37 0x000003ed180c testing::TestCase::Run()
#38 0x000003edd30c testing::internal::UnitTestImpl::RunAllTests()
#39 0x000003cc0d6e _ZN7testing8internal12InvokeHelperIbNSt3__15tupleIJEEEE12InvokeMethodI17TetherServiceTestMS7_FbvEEEbPT_T0_RKS4_
#40 0x000003ee63e2 testing::internal::HandleExceptionsInMethodIfSupported<>()
#41 0x000003edcf4a testing::UnitTest::Run()
#42 0x0000068a4411 RUN_ALL_TESTS()
#43 0x0000068a20be base::TestSuite::Run()
#44 0x000006a29a5d content::UnitTestTestSuite::Run()
#45 0x000000920dcd _ZN4base8internal13FunctorTraitsIMNS_7RunLoopEFvvEvE6InvokeIPS2_JEEEvS4_OT_DpOT0_
#46 0x000000920d14 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMNS_7RunLoopEFvvEJPS4_EEEvOT_DpOT0_
#47 0x00000688c605 _ZN4base8internal7InvokerINS0_9BindStateIMN7content17UnitTestTestSuiteEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEiOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE
#48 0x00000688c54c _ZN4base8internal7InvokerINS0_9BindStateIMN7content17UnitTestTestSuiteEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE3RunEPNS0_13BindStateBaseE
#49 0x00000099b16d _ZNKR4base17RepeatingCallbackIFvvEE3RunEv
#50 0x0000068a7fff base::(anonymous namespace)::LaunchUnitTestsInternal()
#51 0x0000068a7e95 base::LaunchUnitTests()
#52 0x00000688c36c main
#53 0x7f394c98df45 __libc_start_main
#54 0x0000009072f8 <unknown>

 
https://chromium-review.googlesource.com/734801 also seems to be causing a memory leak in tests.
First failing bot run contains that CL: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/24405

(list of builds: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29?numbuilds=100)

Example errors:
Indirect leak of 328 byte(s) in 1 object(s) allocated from:
    #0 0xafae82 in operator new(unsigned long) (/b/s/w/ir/out/Release/unit_tests+0xafae82)
    #1 0xc67568e in chromeos::WallpaperManager::GetPendingWallpaper() chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc:2222:9
    #2 0xc67b7ed in chromeos::WallpaperManager::SetUserWallpaper(AccountId const&) chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc:1855:7
    #3 0xc651b9f in chromeos::ChromeUserManagerImpl::PublicAccountUserLoggedIn(user_manager::User*) chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:863:28
    #4 0x144651d4 in user_manager::UserManagerBase::UserLoggedIn(AccountId const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool) components/user_manager/user_manager_base.cc:171:7
    #5 0x3615ca6 in extensions::(anonymous namespace)::ActiveTabTest_DelegateIsSet_Test::TestBody() chrome/browser/extensions/active_tab_unittest.cc:466:37
    #6 0x7204c6c in testing::Test::Run() third_party/googletest/src/googletest/src/gtest-internal-inl.h
    #7 0x7206a74 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2654:11
    #8 0x7207dd6 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2772:28
    #9 0x721c816 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4677:43
    #10 0x721bd98 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc
    #11 0xf7a2ce5 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46
    #12 0xf7a2ce5 in base::TestSuite::Run() base/test/test_suite.cc:270
    #13 0xf7a830e in Run base/callback.h:92:12
    #14 0xf7a830e in base::(anonymous namespace)::LaunchUnitTestsInternal(base::RepeatingCallback<int ()> const&, unsigned long, int, bool, base::RepeatingCallback<void ()> const&) base/test/launcher/unit_test_launcher.cc:216
    #15 0xf7a7e09 in base::LaunchUnitTests(int, char**, base::RepeatingCallback<int ()> const&) base/test/launcher/unit_test_launcher.cc:475:10
    #16 0xf785b23 in main chrome/test/base/run_all_unittests.cc:30:10
    #17 0x7f092be95f44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287

I'll revert that CL to get the builds green again.
Build turned out green and all MultiUserWindowManagerChromeOSTest passed on first try (not a single mention about the 'Check failed: wallpaper_manager' error). Looking good, let's see if it stays green (and if the rollback also fixes the ASAN tests).

Comment 4 by wzang@chromium.org, Oct 30 2017

Hi tschumann@, can you suggest on how to reproduce the test failure of |MultiUserWindowManagerChromeOSTest| locally? The tests pass on my local build even when the problematic CL is in.

I have an updated CL that I believe fixes the problem but I just want to confirm before checking it in. https://chromium-review.googlesource.com/c/chromium/src/+/745022

Thanks.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 31 2017

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

commit fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2
Author: Wenzhao Zang <wzang@chromium.org>
Date: Tue Oct 31 18:30:24 2017

Make PendingWallpaper pass weak pointers with a default delay

After applying a default delay to PendingWallpaper, some tests
failed ( crbug.com/779504 ) or have memory leak [1].

These tests indirectly called SetUserWallpaper, but they do not
care/know about this, so they do not actively wait until wallpaper
loading is done, but start the destruction directly.

This used to be okay when SetUserWallpaper had zero delay.

But when it has the default delay, the timer is still running when
the destruction starts, and the timer holds a reference to the shared
pointer of PendingWallpaper, which results in memory leak.

What's worse, if the WallpaperManager already destructs itself,
PendingWallpaper::ProcessRequest does not know it, and it crashes.

After changing the shared pointer to a weak pointer, WallpaperManager
will be able to completely manage the lifetime of PendingWallpaper.

[1] https://luci-milo.appspot.com/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20(1)/24405

Below are the description of the original CL:

1) All the PendingWallpaper requests are now subject to a delay
calculated from average loading time. However, in most cases the delay
is still zero, and this change only affects corner cases that multiple
requests coming together, which may result in wallpaper being stuck.

2) Add the account_id update in PendingWallpaper. Did not modify
the functionality of PendingWallpaper, except for some renaming.

TBR=oshima@chromium.org

Bug:  779504 ,  778451 
Change-Id: Iae943f88bbe046849c3afab75f35a253934fe75b
Reviewed-on: https://chromium-review.googlesource.com/745022
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512883}
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service_unittest.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/customization/customization_wallpaper_downloader_browsertest.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/login/lock/views_screen_locker.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/login/ui/user_adding_screen.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/login/ui/webui_login_display.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.h
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager_browsertest.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/ui/ash/lock_screen_client.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.cc
[modify] https://crrev.com/fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2/chrome/browser/ui/webui/chromeos/login/supervised_user_creation_screen_handler.cc

Comment 6 by wzang@chromium.org, Oct 31 2017

For the record, the CL is relanded including a fix (per #5). Please ping this bug if there's additional issue. Thanks.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 1 2017

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

commit 9618fdc418cd5576e0c04e121bd19a84459c1642
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Wed Nov 01 01:37:22 2017

Revert "Make PendingWallpaper pass weak pointers with a default delay"

This reverts commit fd502dfe65e7daa4f37b3dfe16fd013d0e1770f2.

Reason for revert: This patch seems causing interactive_ui_tests failures on Linux Chromium MSan/LSan Tests bots.
https://build.chromium.org/p/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/3892

Original change's description:
> Make PendingWallpaper pass weak pointers with a default delay
> 
> After applying a default delay to PendingWallpaper, some tests
> failed ( crbug.com/779504 ) or have memory leak [1].
> 
> These tests indirectly called SetUserWallpaper, but they do not
> care/know about this, so they do not actively wait until wallpaper
> loading is done, but start the destruction directly.
> 
> This used to be okay when SetUserWallpaper had zero delay.
> 
> But when it has the default delay, the timer is still running when
> the destruction starts, and the timer holds a reference to the shared
> pointer of PendingWallpaper, which results in memory leak.
> 
> What's worse, if the WallpaperManager already destructs itself,
> PendingWallpaper::ProcessRequest does not know it, and it crashes.
> 
> After changing the shared pointer to a weak pointer, WallpaperManager
> will be able to completely manage the lifetime of PendingWallpaper.
> 
> [1] https://luci-milo.appspot.com/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20(1)/24405
> 
> Below are the description of the original CL:
> 
> 1) All the PendingWallpaper requests are now subject to a delay
> calculated from average loading time. However, in most cases the delay
> is still zero, and this change only affects corner cases that multiple
> requests coming together, which may result in wallpaper being stuck.
> 
> 2) Add the account_id update in PendingWallpaper. Did not modify
> the functionality of PendingWallpaper, except for some renaming.
> 
> TBR=oshima@chromium.org
> 
> Bug:  779504 ,  778451 
> Change-Id: Iae943f88bbe046849c3afab75f35a253934fe75b
> Reviewed-on: https://chromium-review.googlesource.com/745022
> Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
> Reviewed-by: Alexander Alekseev <alemate@chromium.org>
> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#512883}

TBR=oshima@chromium.org,alemate@chromium.org,wzang@chromium.org

Change-Id: I62ae0d91bc1cbe953de45a77402f38754de858cf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  779504 ,  778451 
Reviewed-on: https://chromium-review.googlesource.com/748381
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513039}
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service_unittest.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/customization/customization_wallpaper_downloader_browsertest.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/login/lock/views_screen_locker.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/login/ui/user_adding_screen.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/login/ui/webui_login_display.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.h
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager_browsertest.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/ui/ash/lock_screen_client.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.cc
[modify] https://crrev.com/9618fdc418cd5576e0c04e121bd19a84459c1642/chrome/browser/ui/webui/chromeos/login/supervised_user_creation_screen_handler.cc

Comment 8 by wzang@chromium.org, Nov 2 2017

Status: Fixed (was: Available)

Sign in to add a comment