New issue
Advanced search Search tips

Issue 759793 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug

Blocking:
issue 882906
issue 754861



Sign in to add a comment

content_unittests doesn't deinit BrowserThreadImpl properly in some tests

Project Member Reported by scottmg@chromium.org, Aug 28 2017

Issue description

content_unittests doesn't deinit BrowserThreadImpl properly in some tests.

This showed up on Fuchsia but is almost definitely cross-platform.

For example:

$ out/fuch/bin/run_content_unittests --gtest_filter=BrowserMainLoopTest.CreateThreadsInSingleProcess:BrowserThreadTest.PostTask

[00001.762] 03421.03448> Note: Google Test filter = BrowserMainLoopTest.CreateThreadsInSingleProcess:BrowserThreadTest.PostTask
[00001.763] 03421.03448> [==========] Running 2 tests from 2 test cases.
[00001.763] 03421.03448> [----------] Global test environment set-up.
[00001.763] 03421.03448> [----------] 1 test from BrowserMainLoopTest
[00001.763] 03421.03448> [ RUN      ] BrowserMainLoopTest.CreateThreadsInSingleProcess
[00001.775] 03421.03448> [       OK ] BrowserMainLoopTest.CreateThreadsInSingleProcess (12 ms)
[00001.775] 03421.03448> [----------] 1 test from BrowserMainLoopTest (12 ms total)
[00001.775] 03421.03448>
[00001.775] 03421.03448> [----------] 1 test from BrowserThreadTest
[00001.775] 03421.03448> [ RUN      ] BrowserThreadTest.PostTask
[00001.776] 03421.03448> [3:1937305217:0828/195652.515799:1776059:FATAL:browser_thread_impl.cc(349)] Check failed: globals.states[identifier_] == BrowserThreadState::UNINITIALIZED (3 vs. 0)
#00: base::debug::StackTrace::StackTrace(unsigned long) at base/debug/stack_trace_fuchsia.cc:173
#01: base::debug::StackTrace::StackTrace() at base/debug/stack_trace.cc:199
#02: logging::LogMessage::~LogMessage() at base/logging.cc:560
#03: content::BrowserThreadImpl::Initialize() at content/browser/browser_thread_impl.cc:350
#04: content::BrowserThreadImpl::BrowserThreadImpl(content::BrowserThread::ID) at content/browser/browser_thread_impl.cc:159
#05: content::BrowserThreadTest::SetUp() at content/browser/browser_thread_unittest.cc:34
#06: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2399
#07: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2452
#08: testing::Test::Run() at third_party/googletest/src/googletest/src/gtest.cc:2469
#09: pc 0x11e7cc9e9bfd (app:content_unittests,0x6311bfd)
#10: testing::TestCase::Run() at third_party/googletest/src/googletest/src/gtest.cc:2770
#11: testing::internal::UnitTestImpl::RunAllTests() at third_party/googletest/src/googletest/src/gtest.cc:4647
#12: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2399
#13: bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2452
#14: testing::UnitTest::Run() at third_party/googletest/src/googletest/src/gtest.cc:4256
#15: RUN_ALL_TESTS() at third_party/googletest/src/googletest/include/gtest/gtest.h:2237
#16: base::TestSuite::Run() at base/test/test_suite.cc:270
#17: content::UnitTestTestSuite::Run() at content/public/test/unittest_test_suite.cc:45
#18: int base::internal::FunctorTraits<int (content::UnitTestTestSuite::*)(), void>::Invoke<content::UnitTestTestSuite*>(int (content::UnitTestTestSuite::*)(), content::UnitTestTestSuite*&&) at base/bind_internal.h:194
#19: int base::internal::InvokeHelper<false, int>::MakeItSo<int (content::UnitTestTestSuite::* const&)(), content::UnitTestTestSuite*>(int (content::UnitTestTestSuite::* const&)(), content::UnitTestTestSuite*&&) at base/bind_internal.h:263
#20: int base::internal::Invoker<base::internal::BindState<int (content::UnitTestTestSuite::*)(), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int ()>::RunImpl<int (content::UnitTestTestSuite::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > const&, 0ul>(int (content::UnitTestTestSuite::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > const&, std::__1::integer_sequence<unsigned long, 0ul>) at base/bind_internal.h:335
#21: base::internal::Invoker<base::internal::BindState<int (content::UnitTestTestSuite::*)(), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int ()>::Run(base::internal::BindStateBase*) at base/bind_internal.h:317
#22: base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>::Run() const & at base/callback.h:80
#23: base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned long, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) at base/test/launcher/unit_test_launcher.cc:216
#24: base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) at base/test/launcher/unit_test_launcher.cc:475
#25: main at content/test/run_all_unittests.cc:20
#26: pc 0x66fa2961a04e (libc.so,0x1c04e)
[00001.776] 03421.03448>

 
Confirmed this fails in the same way on plain Linux too.

It seems there's a helper in BrowserThreadImpl::ResetGlobalsForTesting, but the "real" shutdown in ShutdownThreadsAndCleanUp() doesn't shutdown the UI thread (understandably?) so then ResetGlobalsForTesting() can't be used on UI. So not totally trivial to fix.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 31 2017

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

commit 80cb2724dc06ba309e0aa9216b92209b7d9893db
Author: Scott Graham <scottmg@chromium.org>
Date: Thu Aug 31 02:12:48 2017

Fix deinit of BrowserThreadImpl in BrowserMainLoopTest.CreateThreadsInSingleProcess

BrowserMainLoop shuts down its BrowserThreadImpls, but SHUTDOWN is a
distinct state from UNINITIALIZED which is what BrowserThreadImpl expects
on initialization in the next test. Use ResetGlobalsForTesting() to
clear up lingering state.

This showed up on Fuchsia, but was cross-platform.

Bug:  759793 ,  754861 
Change-Id: I2ffa5226e92e583ae317b8fb9f31bfce3e8e32ef
Reviewed-on: https://chromium-review.googlesource.com/639332
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498723}
[modify] https://crrev.com/80cb2724dc06ba309e0aa9216b92209b7d9893db/content/browser/browser_main_loop_unittest.cc
[modify] https://crrev.com/80cb2724dc06ba309e0aa9216b92209b7d9893db/testing/buildbot/filters/fuchsia.content_unittests.filter

Status: Fixed (was: Started)
Blocking: 882906

Sign in to add a comment