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

Issue 823858 link

Starred by 1 user
Status: Fixed
Owner:
treib@chromium.org
Closed: Mar 2018
Cc:
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

OneGoogleBarFetcherImplTest is flaky (in DCHECK builds)

Project Member Reported by gab@chromium.org, Mar 20 2018 
OS: Windows (but likely all)

Random (but reliable) crashes in local runs of out\Release\unit_tests.exe --gtest_filter=OneGoogleBarFetcherImplTest*

@treib per OneGoogleBar revision history.

Example failures below:

[ RUN      ] OneGoogleBarFetcherImplTest.HandlesResponsePreamble
[44704:29528:0320/161029.318:714033375:FATAL:observer_list_threadsafe.h(115)] Check failed: !ContainsKey(observers_, observer).
Backtrace:
        base::debug::StackTrace::StackTrace [0x66BEF970+32] (D:\src\chrome\src\base\debug\stack_trace_win.cc:286)
        base::debug::StackTrace::StackTrace [0x66BEF08D+13] (D:\src\chrome\src\base\debug\stack_trace.cc:199)
        logging::LogMessage::~LogMessage [0x66C16FD4+84] (D:\src\chrome\src\base\logging.cc:595)
        base::ObserverListThreadSafe<net::NetworkChangeNotifier::NetworkChangeObserver>::AddObserver [0x688BE72A+146] (D:\src\chrome\src\base\observer_list_threadsafe.h:116)
        net::NetworkChangeNotifier::AddNetworkChangeObserver [0x688BE627+33] (D:\src\chrome\src\net\base\network_change_notifier.cc:882)
        GoogleURLTracker::GoogleURLTracker [0x0363EF07+367] (D:\src\chrome\src\components\google\core\browser\google_url_tracker.cc:45)
        OneGoogleBarFetcherImplTest::OneGoogleBarFetcherImplTest [0x01ED3720+220] (D:\src\chrome\src\chrome\browser\search\one_google_bar\one_google_bar_fetcher_impl_unittest.cc:90)
        OneGoogleBarFetcherImplTest::OneGoogleBarFetcherImplTest [0x01ED3601+39] (D:\src\chrome\src\chrome\browser\search\one_google_bar\one_google_bar_fetcher_impl_unittest.cc:66)
        testing::internal::TestFactoryImpl<OneGoogleBarFetcherImplTest_HandlesResponsePreamble_Test>::CreateTest [0x01ED4AFE+26] (D:\src\chrome\src\third_party\googletest\src\googletest\include\gtest\internal\gtest-internal.h:466)
        testing::TestInfo::Run [0x029975A5+179] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:2652)
        testing::TestCase::Run [0x029979A2+238] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:2778)
        testing::internal::UnitTestImpl::RunAllTests [0x0299DC42+632] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:5034)
        testing::UnitTest::Run [0x0299D8CC+156] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:4652)
        base::TestSuite::Run [0x032DCAF8+104] (D:\src\chrome\src\base\test\test_suite.cc:284)
        base::OnceCallback<int ()>::Run [0x02B9E54F+45] (D:\src\chrome\src\base\callback.h:95)
        base::`anonymous namespace'::LaunchUnitTestsInternal [0x032DEC4F+274] (D:\src\chrome\src\base\test\launcher\unit_test_launcher.cc:225)
        base::LaunchUnitTests [0x032DEB14+176] (D:\src\chrome\src\base\test\launcher\unit_test_launcher.cc:576)
        main [0x04E91460+324] (D:\src\chrome\src\chrome\test\base\run_all_unittests.cc:30)
        __scrt_common_main_seh [0x04E99F29+248] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
        BaseThreadInitThunk [0x767162C4+36]
        RtlSubscribeWnfStateChangeNotification [0x77690F79+1081]
        RtlSubscribeWnfStateChangeNotification [0x77690F44+1028]


[ RUN      ] OneGoogleBarFetcherImplTest.HttpErrorIsFatal
[52800:43612:0320/161053.099:714057156:FATAL:observer_list_threadsafe.h(115)] Check failed: !ContainsKey(observers_, observer).
Backtrace:
        base::debug::StackTrace::StackTrace [0x66C9F970+32] (D:\src\chrome\src\base\debug\stack_trace_win.cc:286)
        base::debug::StackTrace::StackTrace [0x66C9F08D+13] (D:\src\chrome\src\base\debug\stack_trace.cc:199)
        logging::LogMessage::~LogMessage [0x66CC6FD4+84] (D:\src\chrome\src\base\logging.cc:595)
        base::ObserverListThreadSafe<net::NetworkChangeNotifier::NetworkChangeObserver>::AddObserver [0x6896E72A+146] (D:\src\chrome\src\base\observer_list_threadsafe.h:116)
        net::NetworkChangeNotifier::AddNetworkChangeObserver [0x6896E627+33] (D:\src\chrome\src\net\base\network_change_notifier.cc:882)
        GoogleURLTracker::GoogleURLTracker [0x0384EF13+367] (D:\src\chrome\src\components\google\core\browser\google_url_tracker.cc:45)
        OneGoogleBarFetcherImplTest::OneGoogleBarFetcherImplTest [0x020E3720+220] (D:\src\chrome\src\chrome\browser\search\one_google_bar\one_google_bar_fetcher_impl_unittest.cc:90)
        OneGoogleBarFetcherImplTest::OneGoogleBarFetcherImplTest [0x020E3601+39] (D:\src\chrome\src\chrome\browser\search\one_google_bar\one_google_bar_fetcher_impl_unittest.cc:66)
        testing::internal::TestFactoryImpl<OneGoogleBarFetcherImplTest_HttpErrorIsFatal_Test>::CreateTest [0x020E4ECA+26] (D:\src\chrome\src\third_party\googletest\src\googletest\include\gtest\internal\gtest-internal.h:466)
        testing::TestInfo::Run [0x02BA75B1+179] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:2652)
        testing::TestCase::Run [0x02BA79AE+238] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:2778)
        testing::internal::UnitTestImpl::RunAllTests [0x02BADC4E+632] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:5034)
        testing::UnitTest::Run [0x02BAD8D8+156] (D:\src\chrome\src\third_party\googletest\src\googletest\src\gtest.cc:4652)
        base::TestSuite::Run [0x034ECB04+104] (D:\src\chrome\src\base\test\test_suite.cc:284)
        base::OnceCallback<int ()>::Run [0x02DAE55B+45] (D:\src\chrome\src\base\callback.h:95)
        base::`anonymous namespace'::LaunchUnitTestsInternal [0x034EEC5B+274] (D:\src\chrome\src\base\test\launcher\unit_test_launcher.cc:225)
        base::LaunchUnitTests [0x034EEB20+176] (D:\src\chrome\src\base\test\launcher\unit_test_launcher.cc:576)
        main [0x050A1460+324] (D:\src\chrome\src\chrome\test\base\run_all_unittests.cc:30)
        __scrt_common_main_seh [0x050A9F29+248] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
        BaseThreadInitThunk [0x767162C4+36]
        RtlSubscribeWnfStateChangeNotification [0x77690F79+1081]
        RtlSubscribeWnfStateChangeNotification [0x77690F44+1028]

Comment 1 by treib@chromium.org, Mar 21 2018

Status: Started (was: Assigned)
Thanks for reporting!
I think I found the problem: The test instantiates a GoogleURLTracker, which registers itself as a NetworkChangeObserver. It unregisters itself again in its Shutdown(), but the test never calls that.
The duplicate-observer DCHECK then gets triggered during the next test, if the newly-instantiated GoogleURLTracker just so happens to have the same address as before (which in my experience often happens on Windows, but not on other platforms).

The good news is that it'll be easy to fix for this test. The bad news is that probably every other unit test that instantiates a GoogleURLTracker will run into the same problem (but it looks like there aren't any right now).
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 22 2018 

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

commit ba7b02ff0cbe99e37c7a587858412c62a64fb549
Author: Marc Treib <treib@chromium.org>
Date: Thu Mar 22 08:58:41 2018

OneGoogleBarFetcherImplTest: Shutdown() the GoogleURLTracker before destruction

GoogleURLTracker registers itself as a NetworkChangeObserver at the
NetworkChangeNotifier singleton, and unregisters itself again in Shutdown().
In non-test code, GoogleURLTracker is a KeyedService and so its Shutdown()
gets called at the proper time automatically. But in unit tests that
instantiate it directly, we have to explicitly call Shutdown(),
otherwise we leave behind a dangling pointer.

Bug:  823858 
Change-Id: Idc215823ad81c983c8693c04688eccce40f9f26c
Reviewed-on: https://chromium-review.googlesource.com/973603
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545017}
[modify] https://crrev.com/ba7b02ff0cbe99e37c7a587858412c62a64fb549/chrome/browser/search/one_google_bar/one_google_bar_fetcher_impl_unittest.cc
[modify] https://crrev.com/ba7b02ff0cbe99e37c7a587858412c62a64fb549/components/google/core/browser/google_url_tracker.h

Comment 3 by treib@chromium.org, Mar 22 2018

Status: Fixed (was: Started)
That should fix it. Please reopen if it happens again!
►

Sign in to add a comment