New issue
Advanced search Search tips

Issue 674153 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Dec 2016
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK failure from ShillClientHelper::CallDictionaryValueMethod

Project Member Reported by emaxx@chromium.org, Dec 14 2016

Issue description

The DCHECK happens in debug Chrome OS ToT builds:

[FATAL:weak_ptr.cc(26)] Check failed: sequence_checker_.CalledOnValidSequence(). WeakPtrs must be checked on the same sequenced thread.
#0 0x7fa695dcb976 base::debug::StackTrace::StackTrace()
#1 0x7fa695ded97e logging::LogMessage::~LogMessage()
#2 0x7fa695df5fd1 base::internal::WeakReference::Flag::IsValid()
#3 0x7fa696d2de3b chromeos::ShillClientHelper::CallDictionaryValueMethod()
#4 0x7fa696cfd55e chromeos::(anonymous namespace)::ShillManagerClientImpl::GetNetworksForGeolocation()
#5 0x7fa696c3896f chromeos::GeolocationHandler::RequestWifiAccessPoints()
#6 0x7fa696c38fa0 chromeos::GeolocationHandler::GetWifiAccessPoints()
#7 0x7fa697be2960 device::WifiDataProviderChromeOs::GetAccessPointData()
#8 0x7fa697be30d8 device::WifiDataProviderChromeOs::DoWifiScanTaskOnUIThread()
#9 0x7fa697be3223 device::WifiDataProviderChromeOs::DoStartTaskOnUIThread()
#10 0x7fa695e926bb base::debug::TaskAnnotator::RunTask()
#11 0x7fa695df6aa2 base::MessageLoop::RunTask()
#12 0x7fa695df6f9e base::MessageLoop::DeferOrRunPendingTask()
#13 0x7fa695dfafb0 base::MessageLoop::DoWork()
#14 0x7fa695dfb79c base::MessagePumpDefault::Run()
#15 0x7fa695df80be base::MessageLoop::RunHandler()
#16 0x7fa695e2d35b base::RunLoop::Run()
#17 0x7fa695e5a1dd base::Thread::Run()
#18 0x7fa695e5a422 base::Thread::ThreadMain()
#19 0x7fa695e537bc base::(anonymous namespace)::ThreadFunc()
#20 0x7fa6924cb307 <unknown>
#21 0x7fa69121f5cd clone


It's not obvious whether the problematic weak pointer comes from ShillClientHelper or from GeolocationHandler. I don't see any relevant recent changes in both places.
However, the DCHECK in WeakReference itself is likely to be introduced later than all this code was written.
 
Labels: -Pri-2 Pri-1
This is very strange, especially that it would start occurring recently (although, in practice not a lot of devs run Debug builds).

Can you reproduce this?

If so, can you add some thread checks up the stack? All of this should be running on the UI thread. If that is not the case, that is the problem. If that is the case, somehow ShillClientHelper must not be getting constructed on the UI thread, in which case we should test for that somehow.

Labels: M-57
BTW, in what environment are you running into this? What does your args.gn look like?

Comment 4 Deleted

Comment 5 by emaxx@chromium.org, Dec 15 2016

Steven, yes, the issue is reproduced reliably for me.
I'm using the typical environment, except with an added "dchecks_always_on = true" setting. The image is being run on a Pixel.

I didn't understand comment #4 - did you post this into a wrong bug?

I'd try to add now more thread checks.
Yes, comment #4 was for the wrong issue, sorry for the confusion.

Are you building with Simple Chrome, or in the chroot?

How exactly do you reproduce this? So far I have been unable to reproduce it on a samus device, but at the moment I am hitting a different DCHECK that I am looking into.


Comment 7 by emaxx@chromium.org, Dec 15 2016

I'm building in chroot (with the automatic, latest Chrome OS SDK), following these instructions:
https://www.chromium.org/chromium-os/how-tos-and-troubleshooting/building-chromium-browser

> How exactly do you reproduce this? So far I have been unable to
> reproduce it on a samus device, but at the moment I am hitting a
> different DCHECK that I am looking into.
This happens to me shortly after logging into a user session.
BTW, I also met other DCHECKs that I had to stub out (I haven't triaged them yet).
Status: Started (was: Assigned)
OK, I've been able to reproduce this locally using a Simple Chrome:

$ gn gen out_$SDK_BOARD/Release --args="${GN_ARGS} dcheck_always_on = true"
$ deploy_chrome --build-dir=out_${SDK_BOARD}/Release --to 100.106.137.122 --nostrip --target-dir=/usr/local/chrome --mount-dir=/opt/google/chrome

To repro:
* Visit google.com and search for 'restaurants'. 
* This will prompt you to allow google.com to "Know your location".
* Click 'Allow'.
* Observer: crash


Project Member

Comment 9 by bugdroid1@chromium.org, Dec 17 2016

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

commit 43c6a582d94fe6ec49330636c819601650f63a24
Author: stevenjb <stevenjb@chromium.org>
Date: Sat Dec 17 00:37:11 2016

WifiDataProviderChromeOs: Call NetworkHandler from correct task runner.

At some point, WifiDataProviderChromeOs construction changed to the
GeolocationProviderImpl thread, making the following code in its
constructor incorrect:

main_task_runner_(base::ThreadTaskRunnerHandle::Get())

Rather than relying on a "main" task runner for NetworkHandger calls,
use NetworkHandler::Get()->task_runner() instead.

BUG= 674153 , 661304

Review-Url: https://codereview.chromium.org/2584733002
Cr-Commit-Position: refs/heads/master@{#439253}

[modify] https://crrev.com/43c6a582d94fe6ec49330636c819601650f63a24/device/geolocation/wifi_data_provider_chromeos.cc
[modify] https://crrev.com/43c6a582d94fe6ec49330636c819601650f63a24/device/geolocation/wifi_data_provider_chromeos.h

Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 20 2017

Labels: merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c7e0db8fdd332bda882e134f8f58b73a7a6f0aa

commit 4c7e0db8fdd332bda882e134f8f58b73a7a6f0aa
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Fri Jan 20 17:29:47 2017

WifiDataProviderChromeOs: Call NetworkHandler from correct task runner.

At some point, WifiDataProviderChromeOs construction changed to the
GeolocationProviderImpl thread, making the following code in its
constructor incorrect:

main_task_runner_(base::ThreadTaskRunnerHandle::Get())

Rather than relying on a "main" task runner for NetworkHandger calls,
use NetworkHandler::Get()->task_runner() instead.

BUG= 674153 , 661304

Review-Url: https://codereview.chromium.org/2584733002
Cr-Commit-Position: refs/heads/master@{#439253}
(cherry picked from commit 43c6a582d94fe6ec49330636c819601650f63a24)

Review-Url: https://codereview.chromium.org/2645133002 .
Cr-Commit-Position: refs/branch-heads/2924@{#818}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/4c7e0db8fdd332bda882e134f8f58b73a7a6f0aa/device/geolocation/wifi_data_provider_chromeos.cc
[modify] https://crrev.com/4c7e0db8fdd332bda882e134f8f58b73a7a6f0aa/device/geolocation/wifi_data_provider_chromeos.h

Labels: VerifyIn-61

Comment 13 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment