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

Issue 746478 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

EarlGrey tests hit a DCHECK during shutdown

Project Member Reported by sdefresne@chromium.org, Jul 19 2017

Issue description

All EarlGrey tests hit a DCHECK during shutdown:

Thread 0 Crashed:: CrWebMain  Dispatch queue: com.apple.main-thread
0   org.chromium.gtest.generic-unit-test	0x0000000105948534 base::debug::BreakDebugger() + 20
1   org.chromium.gtest.generic-unit-test	0x00000001059aff38 logging::LogMessage::~LogMessage() + 4664
2   org.chromium.gtest.generic-unit-test	0x00000001059aca55 logging::LogMessage::~LogMessage() + 21
3   org.chromium.gtest.generic-unit-test	0x0000000106d10fd2 net::HistogramWatcher::~HistogramWatcher() + 258
4   org.chromium.gtest.generic-unit-test	0x0000000106d104f5 net::HistogramWatcher::~HistogramWatcher() + 21
5   org.chromium.gtest.generic-unit-test	0x0000000106d10519 net::HistogramWatcher::~HistogramWatcher() + 25
6   org.chromium.gtest.generic-unit-test	0x0000000106d026a1 net::NetworkChangeNotifier::~NetworkChangeNotifier() + 977
7   org.chromium.gtest.generic-unit-test	0x00000001076c712b net::NetworkChangeNotifierMac::~NetworkChangeNotifierMac() + 987
8   org.chromium.gtest.generic-unit-test	0x00000001076c71f5 net::NetworkChangeNotifierMac::~NetworkChangeNotifierMac() + 21
9   org.chromium.gtest.generic-unit-test	0x00000001076c7219 net::NetworkChangeNotifierMac::~NetworkChangeNotifierMac() + 25
10  org.chromium.gtest.generic-unit-test	0x000000010554df30 std::__1::unique_ptr<net::NetworkChangeNotifier, std::__1::default_delete<net::NetworkChangeNotifier> >::~unique_ptr() + 176
11  libsystem_c.dylib             	0x00000001187f6d3c __cxa_finalize_ranges + 332
12  libsystem_c.dylib             	0x00000001187f70a1 exit + 48

Example: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.mac%2Fios-simulator%2F260754%2F%2B%2Frecipes%2Fsteps%2Fios_chrome_integration_egtests__iPhone_6s_iOS_10.0_%2F0%2Flogs%2Fcrash_report__2017-07-18-081112_%2F0

This was introduced by https://chromium-review.googlesource.com/558268 as it changes when NetworkChangeNotifier and MessageLoopForUI are destroyed (where destroyed as part of destruction of WebMainLoop but are no destroyed after returning from main).

rohitrao: the bots are still green, which I think is an issue, they should have turned red IMO. Do you have an idea of why they are not failing?
 
I also think the bots should be red.
I think they stay green because the crash is outside of test suite, and specially after the 

Test Case '-[WebUITestCase testChromeURLsLoadWithoutError]' passed (3.424 seconds).
Test Suite 'WebUITestCase' passed at 2017-07-19 09:56:08.652.
	 Executed 5 tests, with 0 failures (0 unexpected) in 9.832 (9.844) seconds
Test Suite 'ios_chrome_ui_egtests.app' passed at 2017-07-19 09:56:08.654.
	 Executed 183 tests, with 0 failures (0 unexpected) in 1351.329 (1352.557) seconds
Test Suite 'All tests' passed at 2017-07-19 09:56:08.657.
	 Executed 183 tests, with 0 failures (0 unexpected) in 1351.329 (1352.561) seconds

message
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 20 2017

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

commit 2bb1753001e50e1952be92f39f0557699d4ca727
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Thu Jul 20 08:10:29 2017

Cleanup object destruction order during application shutdown.

The NetworkChangeNotifier owns an object that checks it is destroyed
on the correct thread (HistogramWatcher). This requires the object
to be destroyed before the thread is stopped, so add helper function
to control when the NetworkChangeNotifier is destroyed.

For the same reason, do it also for MessageLoopForUI.

Call the new function from WebMainLoop destructor. This fixes a new
regression that was introduced during the refactoring of this code
that moved the object destruction till after the "main" function
returns (as they are owned by "static" globals).

To ensure the destruction happens in the same order as before the
refactoring in https://chromium-review.googlesource.com/558268,
use base::ScopedClosureRunner to register in which order the helper
functions are called.

As CroNet code was leaking those objects, do not add a call to the
destroy function there.

Bug:  746478 
Change-Id: If2e5d1dea15c73bccd973222f9811660a5687b77
Reviewed-on: https://chromium-review.googlesource.com/577851
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488164}
[modify] https://crrev.com/2bb1753001e50e1952be92f39f0557699d4ca727/ios/web/app/web_main_loop.h
[modify] https://crrev.com/2bb1753001e50e1952be92f39f0557699d4ca727/ios/web/app/web_main_loop.mm
[modify] https://crrev.com/2bb1753001e50e1952be92f39f0557699d4ca727/ios/web/public/global_state/ios_global_state.h
[modify] https://crrev.com/2bb1753001e50e1952be92f39f0557699d4ca727/ios/web/public/global_state/ios_global_state.mm

Status: Fixed (was: Started)

Sign in to add a comment