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

Issue 779144 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK failing in host_resolver_impl.cc

Project Member Reported by bsheedy@chromium.org, Oct 27 2017

Issue description

The following DCHECK is occasionally (5-10% of the time) getting hit when trying to locally run a test https://cs.chromium.org/chromium/src/net/dns/host_resolver_impl.cc?q=host_resolver_impl.cc&sq=package:chromium&dr&l=2494

It was added as part of https://chromium-review.googlesource.com/c/chromium/src/+/723596. Attached is a symbolized stack trace.

This was reproducing on a 2016 Pixel running 8.0 OPR1.170623.027 userdebug.

The particular test that's failing is WebVrInputTest#testControllerClicksRegisteredOnDaydream, although it's quite possible this happens on other tests (there's nothing special about that test's startup). VR tests require a bit of extra setup, and this particular test only works on Daydream-ready devices (essentially, Pixel and Pixel 2's/XL's).

You'll need to download the extra VR APK with:
export DOWNLOAD_VR_TEST_APKS=1 && gclient runhooks

Then build the "chrome_public_test_vr_apk" target like normal. The gn args I used are:
is_component_build = false
is_debug = true
proprietary_codecs = true
strip_absolute_paths_from_debug_symbols = true
symbol_level = 1
target_os = "android"
use_goma = true
ffmpeg_branding = "Chrome"
include_vr_data = true

To run the test, use the following (assumes you built in out/Debug and are currently in src/):
out/Debug/bin/run_chrome_public_test_vr_apk --local-output --json-results-file testoutputjson.json --test-filter WebVrInputTest#testControllerClicksRegisteredOnDaydream --repeat 10 --additional-apk third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk --shared-prefs-file chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json

--local-output and --json-results-file are to produce a local dashboard that allows you to get logcat output (the instrumentation test runner automatically clears it between test runs)

--additional-apk and --shared-prefs-file setup the device to work for VR tests. They can be omitted on subsequent runs.
 
host_resolver_impl_dcheck.txt
32.8 KB View Download

Comment 1 by mge...@chromium.org, Oct 27 2017

Components: Internals>Network>DNS
Labels: OS-Android

Comment 2 by mge...@chromium.org, Oct 31 2017

I understand what the race is:
- On the DnsConfigService thread, the DnsConfigService is created and does its initial read, and posts tasks to observers
- On the IO thread, the DnsClient is created and reads the config (https://cs.chromium.org/chromium/src/net/dns/host_resolver_impl.cc?dr=CSs&l=2559)
- On the IO thread, the posted task from earlier runs, and calls UpdateDNSConfig

My guess is this has only been reported in the VR tests because it only happens on particularly fast devices or something, and the rest of us don't test with Pixels.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31 2017

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

commit 668e8441703bde5dd5ab92ce7446898c709509fc
Author: Miriam Gershenson <mgersh@chromium.org>
Date: Tue Oct 31 20:31:53 2017

Fix bad DCHECK when reading DNS config

The original version of this DCHECK missed a case. It's possible for
SetDnsClient() to run after NetworkChangeNotifier::SetInitialDnsConfig()
but before the task it posts to DNSObservers. When that happens,
DnsClient will already have a valid DnsConfig by the time
UpdateDNSConfig() runs. NetworkChangeNotifier's DNSConfig can't have
changed again since then because there are too many PostTasks in the
way.

Bug:  779144 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I9acafc463b9ee9dbcc7c7c08da32d1d86901f105
Reviewed-on: https://chromium-review.googlesource.com/747503
Reviewed-by: Julia Tuttle <juliatuttle@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512940}
[modify] https://crrev.com/668e8441703bde5dd5ab92ce7446898c709509fc/net/dns/host_resolver_impl.cc

Comment 4 by mge...@chromium.org, Oct 31 2017

Status: Fixed (was: Assigned)
Labels: VR-Caught-By-Test
Labels: -VR-Caught-By-Test XR-Caught-By-Test

Sign in to add a comment