Use of MockNetworkChangeNotifier can create second NetworkChangeNotifier |
||
Issue descriptionNetworkChangeNotifier is a singleton and the implementation relies on a global that is set in the NetworkChangeNotifier constructor, and cleared in ~NetworkChangeNotifier(). MockNetworkChangeNotifier inherits from NetworkChangeNotifier so it will set g_network_change_notifier if constructed. The running of unit_tests can fail if an instance of NetworkChangeNotifier exists and then a MockNetworkChangeNotifier is instantiated which fails a DCHECK. This seems to reliability reproduce this failure: ``` out/Debug/unit_tests --brave-new-test-launcher --test-launcher-jobs=2 --gtest_filter=':SiteDataSizeCollectorTest.FetchFileSystem:TabSpecificContentSettingsTest.BlockedFileSystems:' ``` One possible solution would be to convert NetworkChangeNotifier to an interface and the current NetworkChangeNotifier into NetworkChangeNotifierImpl.
,
Jul 16
Can you explain more about what you're doing to induce the failures? Are you running tests in parallel? If so, how are static global APIs meant to work?
,
Jan 8
tests don't need to be run in parallel; two unit test cases run in sequence can hit this because the global variable persists between runs. In an example I found (ChromeLauncherControllerDemoModeTest.*), a real NetworkChangeNotifier is being created during base::RunLoop::RunUntilIdle() called from BrowserWithTestWindowTest::TearDown(). The next test creates a MockNetworkChangeNotifier, when the real one is already created. https://chromium-review.googlesource.com/c/chromium/src/+/1399744 Additional test environment: CHROME_DEVEL_SANDBOX=/usr/local/sbin/chrome-devel-sandbox CHROME_HEADLESS=1 LANG=en_US.UTF-8 Command: out_cros/rel/unit_tests --gtest_filter=ChromeLauncher*DemoMode*PinnedApps* IMPORTANT DEBUGGING NOTE: batches of tests are run inside their own process. For debugging a test inside a debugger, use the --gtest_filter=<your_test_name> flag along with --single-process-tests. Using sharding settings from environment. This is shard 0/1 Using 1 parallel jobs. Note: Google Test filter = ChromeLauncherControllerDemoModeTest.PinnedAppsOnline:ChromeLauncherControllerDemoModeTest.PinnedAppsOffline [==========] Running 2 tests from 1 test case. [----------] Global test environment set-up. [----------] 2 tests from ChromeLauncherControllerDemoModeTest [ RUN ] ChromeLauncherControllerDemoModeTest.PinnedAppsOnline [87494:87494:0107/163123.467369:10042101930623:ERROR:render_widget_host_view_base.cc(194)] Not implemented reached in virtual uint32_t content::RenderWidgetHostViewBase::GetCaptureSequenceNumber() const [87494:87494:0107/163123.470437:10042101933690:WARNING:shelf_button.cc(363)] An icon of size 32x32is being scaled up and will look blurry. [87494:87494:0107/163123.516033:10042101979288:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown. [ OK ] ChromeLauncherControllerDemoModeTest.PinnedAppsOnline (204 ms) [ RUN ] ChromeLauncherControllerDemoModeTest.PinnedAppsOffline [87494:87494:0107/163123.606405:10042102069663:FATAL:network_change_notifier.cc(676)] Check failed: !g_network_change_notifier. #0 0x7f635087b1cf base::debug::StackTrace::StackTrace() #1 0x7f63507a997a logging::LogMessage::~LogMessage() #2 0x7f6352cf64d6 net::NetworkChangeNotifier::NetworkChangeNotifier() #3 0x5628988a8ab5 net::test::MockNetworkChangeNotifier::MockNetworkChangeNotifier() #4 0x56289661af13 ChromeLauncherControllerDemoModeTest_PinnedAppsOffline_Test::TestBody() #5 0x5628977bf661 testing::Test::Run() #6 0x5628977c02af testing::TestInfo::Run() #7 0x5628977c0807 testing::TestCase::Run() #8 0x5628977cc197 testing::internal::UnitTestImpl::RunAllTests() #9 0x5628977cbd17 testing::UnitTest::Run() #10 0x5628987efea9 base::TestSuite::Run() #11 0x56289887ef12 content::UnitTestTestSuite::Run() #12 0x5628987f1fad base::(anonymous namespace)::LaunchUnitTestsInternal() #13 0x5628987f1e11 base::LaunchUnitTests() #14 0x5628987e473c main #15 0x7f634348c2b1 __libc_start_main #16 0x56289592dd5a _start [87486:87493:0107/163123.972911:10042102436586:ERROR:kill_posix.cc(84)] Unable to terminate process group 87494: No such process (3) [1/2] ChromeLauncherControllerDemoModeTest.PinnedAppsOnline (204 ms) [2/2] ChromeLauncherControllerDemoModeTest.PinnedAppsOffline (CRASHED) 1 test crashed: ChromeLauncherControllerDemoModeTest.PinnedAppsOffline (../../chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:4547) Tests took 1 seconds.
,
Jan 9
Where do we post tasks that create a real NetworkChangeNotifier that doesn't run until BrowserWithTestWindowTest::TearDown()?
,
Jan 9
Got a stack trace from ChromeLauncherControllerDemoModeTest.PinnedAppsOnline: #0 0x7f2fc9d511cf base::debug::StackTrace::StackTrace() #1 0x7f2fcc1cc555 net::NetworkChangeNotifier::NetworkChangeNotifier() #2 0x7f2fcc1cf2bb net::NetworkChangeNotifierChromeos::NetworkChangeNotifierChromeos() #3 0x7f2fcc1c9c8d net::NetworkChangeNotifier::Create() #4 0x7f2fc5d359af _ZN7network14NetworkServiceC1ENSt4__Cr10unique_ptrIN15service_manager22BinderRegistryWithArgsIJEEENS1_14default_deleteIS5_EEEEN4mojo16InterfaceRequestINS_5mojom14NetworkServiceEEEPN3net6NetLogENSA_INS3_5mojom7ServiceEEE #5 0x7f2fc7d3deb4 content::GetNetworkServiceImpl() #6 0x7f2fc7f42022 content::StoragePartitionImpl::NetworkContextOwner::Initialize() #7 0x7f2fc7f44ee2 _ZN4base8internal7InvokerINS0_9BindStateIMN7content20StoragePartitionImpl19NetworkContextOwnerEFvN4mojo16InterfaceRequestIN7network5mojom14NetworkContextEEE13scoped_refptrIN3net23URLRequestContextGetterEEEJNS0_17UnretainedWrapperIS5_EESB_SF_EEEFvvEE7RunOnceEPNS0_13BindStateBaseE #8 0x7f2fc9c63d51 base::debug::TaskAnnotator::RunTask() #9 0x7f2fc9cf318a base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl() #10 0x7f2fc9cf3733 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoDelayedWork() #11 0x7f2fc9d7460d base::MessagePumpLibevent::Run() #12 0x7f2fc9cf395c base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run() #13 0x7f2fc9cbfc59 base::RunLoop::Run() #14 0x7f2fc9cc028a base::RunLoop::RunUntilIdle() #15 0x55c0beb307be BrowserWithTestWindowTest::TearDown() #16 0x55c0bff372af testing::TestInfo::Run() #17 0x55c0bff37807 testing::TestCase::Run() #18 0x55c0bff43197 testing::internal::UnitTestImpl::RunAllTests() #19 0x55c0bff42d17 testing::UnitTest::Run() #20 0x55c0c0f66ea9 base::TestSuite::Run() #21 0x55c0c0ff5f12 content::UnitTestTestSuite::Run() #22 0x55c0c0f68fad base::(anonymous namespace)::LaunchUnitTestsInternal() #23 0x55c0c0f68e11 base::LaunchUnitTests() #24 0x55c0c0f5b73c main #25 0x7f2fbc9622b1 __libc_start_main I can't really make heads or tails of that, though.
,
Jan 10
Reposting that stack with a couple things run through c++filt: #0 0x7f2fc9d511cf base::debug::StackTrace::StackTrace() #1 0x7f2fcc1cc555 net::NetworkChangeNotifier::NetworkChangeNotifier() #2 0x7f2fcc1cf2bb net::NetworkChangeNotifierChromeos::NetworkChangeNotifierChromeos() #3 0x7f2fcc1c9c8d net::NetworkChangeNotifier::Create() #4 0x7f2fc5d359af network::NetworkService::NetworkService(std::__Cr::unique_ptr<service_manager::BinderRegistryWithArgs<>, std::__Cr::default_delete<service_manager::BinderRegistryWithArgs<> > >, mojo::InterfaceRequest<network::mojom::NetworkService>, net::NetLog*, mojo::InterfaceRequest<service_manager::mojom::Service>) #5 0x7f2fc7d3deb4 content::GetNetworkServiceImpl() #6 0x7f2fc7f42022 content::StoragePartitionImpl::NetworkContextOwner::Initialize() #7 0x7f2fc7f44ee2 base::internal::Invoker<base::internal::BindState<void (content::StoragePartitionImpl::NetworkContextOwner::*)(mojo::InterfaceRequest<network::mojom::NetworkContext>, scoped_refptr<net::URLRequestContextGetter>), base::internal::UnretainedWrapper<content::StoragePartitionImpl::NetworkContextOwner>, mojo::InterfaceRequest<network::mojom::NetworkContext>, scoped_refptr<net::URLRequestContextGetter> >, void ()>::RunOnce(base::internal::BindStateBase*) #8 0x7f2fc9c63d51 base::debug::TaskAnnotator::RunTask() #9 0x7f2fc9cf318a base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl() #10 0x7f2fc9cf3733 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoDelayedWork() #11 0x7f2fc9d7460d base::MessagePumpLibevent::Run() #12 0x7f2fc9cf395c base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run() #13 0x7f2fc9cbfc59 base::RunLoop::Run() #14 0x7f2fc9cc028a base::RunLoop::RunUntilIdle() #15 0x55c0beb307be BrowserWithTestWindowTest::TearDown() #16 0x55c0bff372af testing::TestInfo::Run() #17 0x55c0bff37807 testing::TestCase::Run() #18 0x55c0bff43197 testing::internal::UnitTestImpl::RunAllTests() #19 0x55c0bff42d17 testing::UnitTest::Run() #20 0x55c0c0f66ea9 base::TestSuite::Run() #21 0x55c0c0ff5f12 content::UnitTestTestSuite::Run() #22 0x55c0c0f68fad base::(anonymous namespace)::LaunchUnitTestsInternal() #23 0x55c0c0f68e11 base::LaunchUnitTests() #24 0x55c0c0f5b73c main #25 0x7f2fbc9622b1 __libc_start_main
,
Jan 10
Looks like some code is creating a NetworkService during TearDown which seems suboptimal. My first impression is that this is the real cause of the second NCN being created. |
||
►
Sign in to add a comment |
||
Comment 1 by mmenke@chromium.org
, Jul 13