New issue
Advanced search Search tips

Issue 887953 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 21
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: ----



Sign in to add a comment

chromeos_unittests failing on chromium.memory/Linux Chromium OS ASan LSan Tests (1)

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Sep 21

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of liberato@chromium.org

chromeos_unittests failing on chromium.memory/Linux Chromium OS ASan LSan Tests (1)

Builders failed on: 
- Linux Chromium OS ASan LSan Tests (1): 
  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29


 
Labels: OS-Chrome
Owner: nohle@chromium.org
Status: Assigned (was: Available)
nohle@: i think https://chromium-review.googlesource.com/1237536 may have introduced a new problem.

there are many failures with stacks like this:

==4946==ERROR: AddressSanitizer: heap-use-after-free on address 0x613000000ad8 at pc 0x000004deb504 bp 0x7fffa0a86b90 sp 0x7fffa0a86b88
READ of size 8 at 0x613000000ad8 thread T0
    #0 0x4deb503 in begin buildtools/third_party/libc++/trunk/include/vector:1471:30
    #1 0x4deb503 in base::ObserverList<chromeos::device_sync::DeviceSyncClient::Observer, false, true, base::internal::UncheckedObserverAdapter>::RemoveObserver(chromeos::device_sync::DeviceSyncClient::Observer const*) base/observer_list.h:272
    #2 0xa3e2d8a in ~DeviceReenroller chromeos/services/multidevice_setup/device_reenroller.cc:94:24
    #3 0xa3e2d8a in chromeos::multidevice_setup::DeviceReenroller::~DeviceReenroller() chromeos/services/multidevice_setup/device_reenroller.cc:93
    #4 0x4fc41cb in operator() buildtools/third_party/libc++/trunk/include/memory:2321:5
    #5 0x4fc41cb in reset buildtools/third_party/libc++/trunk/include/memory:2634
    #6 0x4fc41cb in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2588
    #7 0x4fc41cb in ~MultiDeviceSetupDeviceReenrollerTest chromeos/services/multidevice_setup/device_reenroller_unittest.cc:22
    #8 0x4fc41cb in chromeos::multidevice_setup::MultiDeviceSetupDeviceReenrollerTest_IfOnEnrollmentFinishedCalledWithAgreementThenNoReenrollment_Test::~MultiDeviceSetupDeviceReenrollerTest_IfOnEnrollmentFinishedCalledWithAgreementThenNoReenrollment_Test() chromeos/services/multidevice_setup/device_reenroller_unittest.cc:396
    #9 0x1dbb84e in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc
    #10 0x1dbb84e in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2703
    #11 0x1dbcac6 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2816:28
    #12 0x1de4536 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5182:43
    #13 0x1de3785 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc
    #14 0x1de3785 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4791
    #15 0x551b1da in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2333:46
    #16 0x551b1da in base::TestSuite::Run() base/test/test_suite.cc:295
    #17 0x5523e67 in Run base/callback.h:99:12
    #18 0x5523e67 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:225
    #19 0x5523910 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:576:10
    #20 0xd8d6d6 in main chromeos/run_all_unittests.cc:15:10
    #21 0x7f9404c5cf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287

Status: Started (was: Assigned)
liberato@: thanks for the heads-up. Sorry about all of the churn. I'm reverting the change, while I look into it.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 21

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21

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

commit 135c87489dd68bba667c8b4464084aa089df84c7
Author: Josh Nohle <nohle@google.com>
Date: Fri Sep 21 16:37:57 2018

[CrOS MultiDevice] DeviceReenroller: Reorder unit test member variables to fix ASAN build

Because DeviceReenroller now uses the DeviceSyncClient in the destructor
to remove itself as a DeviceSyncClient::Observer, DeviceReenroller needs
to be destroyed before the DeviceSyncClient in the unit tests.

Bug:  887953 ,  887557 ,  887758 
Change-Id: I23e8f150ef08e6ad3d388f4ed86d47123077f71a
Tested: Unit tests with ASAN on
Reviewed-on: https://chromium-review.googlesource.com/1238818
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593220}
[modify] https://crrev.com/135c87489dd68bba667c8b4464084aa089df84c7/chromeos/services/multidevice_setup/device_reenroller_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 21

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

commit 904e9c784fe511c608384c53e50e191b213865df
Author: Frank Liberato <liberato@chromium.org>
Date: Fri Sep 21 18:20:25 2018

Revert "[Build Sheriff] Disable various tests pending fix."

This reverts commit 6e1bb98adba8c572bdfc7734c8a34bbb2d2eb16e.

Reason for revert: https://chromium-review.googlesource.com/1238859 reverted the original CL, tests pass again.

Original change's description:
> [Build Sheriff] Disable various tests pending fix.
> 
> NOTRY=true
> TBR=nohle@chromium.org, dtrainor@chromium.org
> 
> Bug:  887953 
> Change-Id: I3a3f3ea828c86873fdea72d75c50a83a83271408
> Reviewed-on: https://chromium-review.googlesource.com/1238377
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Commit-Queue: Frank Liberato <liberato@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593189}

TBR=liberato@chromium.org

Change-Id: Idf5e5d75ed6087c52d6b70e0e51126ceec000a27
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  887953 
Reviewed-on: https://chromium-review.googlesource.com/1239257
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593262}
[modify] https://crrev.com/904e9c784fe511c608384c53e50e191b213865df/chrome/browser/download/notification/download_notification_interactive_uitest.cc
[modify] https://crrev.com/904e9c784fe511c608384c53e50e191b213865df/chrome/browser/ui/keyboard_lock_interactive_browsertest.cc

Status: Fixed (was: Started)
As mentioned in crbug/887557. Verified that chromeos_unittests turned green after the latest patch: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20(1)/29106

Marking as fixed.
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 24

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

commit dd2cdd6d64c45b598e438def6c561faca7bd1f10
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Sep 24 21:34:45 2018

[CrOS MultiDevice] DeviceReenroller: Reorder unit test member variables to fix ASAN build

Because DeviceReenroller now uses the DeviceSyncClient in the destructor
to remove itself as a DeviceSyncClient::Observer, DeviceReenroller needs
to be destroyed before the DeviceSyncClient in the unit tests.

TBR=nohle@google.com

(cherry picked from commit 135c87489dd68bba667c8b4464084aa089df84c7)

Bug:  887953 ,  887557 ,  887758 
Change-Id: I23e8f150ef08e6ad3d388f4ed86d47123077f71a
Tested: Unit tests with ASAN on
Reviewed-on: https://chromium-review.googlesource.com/1238818
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593220}
Reviewed-on: https://chromium-review.googlesource.com/1241753
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#609}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/dd2cdd6d64c45b598e438def6c561faca7bd1f10/chromeos/services/multidevice_setup/device_reenroller_unittest.cc

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/dd2cdd6d64c45b598e438def6c561faca7bd1f10

Commit: dd2cdd6d64c45b598e438def6c561faca7bd1f10
Author: khorimoto@google.com
Commiter: khorimoto@chromium.org
Date: 2018-09-24 21:34:45 +0000 UTC

[CrOS MultiDevice] DeviceReenroller: Reorder unit test member variables to fix ASAN build

Because DeviceReenroller now uses the DeviceSyncClient in the destructor
to remove itself as a DeviceSyncClient::Observer, DeviceReenroller needs
to be destroyed before the DeviceSyncClient in the unit tests.

TBR=nohle@google.com

(cherry picked from commit 135c87489dd68bba667c8b4464084aa089df84c7)

Bug:  887953 ,  887557 ,  887758 
Change-Id: I23e8f150ef08e6ad3d388f4ed86d47123077f71a
Tested: Unit tests with ASAN on
Reviewed-on: https://chromium-review.googlesource.com/1238818
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593220}
Reviewed-on: https://chromium-review.googlesource.com/1241753
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#609}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}

Sign in to add a comment