"PolicyPrefsTest.PolicyToPrefsMapping" is flaky |
|||||||
Issue description"PolicyPrefsTest.PolicyToPrefsMapping" is flaky. This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label. We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyLwsSBUZsYWtlIiRQb2xpY3lQcmVmc1Rlc3QuUG9saWN5VG9QcmVmc01hcHBpbmcM. Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
,
Mar 19 2018
It looks that the problem is in DCHECK in network annotation. Error message 5676:6396:0316/032955.663:FATAL:network_traffic_annotation.h(226)] Check failed: is_valid(). Backtrace: base::debug::StackTrace::StackTrace [0x00007FF63E2BA6E4+36] logging::LogMessage::~LogMessage [0x00007FF63E211A93+99] net::MutableNetworkTrafficAnnotationTag::operator net::NetworkTrafficAnnotationTag [0x00007FF63C5468C2+98] net::InitSocketHandleForHttpRequest [0x00007FF63E5FE657+2167] net::InitSocketHandleForHttpRequest [0x00007FF63E5FDF6C+396] net::HttpStreamFactoryImpl::Job::DoInitConnectionImpl [0x00007FF63E5F6405+2885] net::HttpStreamFactoryImpl::Job::DoInitConnection [0x00007FF63E5F4752+34] net::HttpStreamFactoryImpl::Job::DoLoop [0x00007FF63E5F4060+640] net::HttpStreamFactoryImpl::Job::RunLoop [0x00007FF63E5F1EC0+96] net::HttpStreamFactoryImpl::Job::StartInternal [0x00007FF63E5F1C15+133] net::HttpStreamFactoryImpl::JobController::DoCreateJobs [0x00007FF63E5FC6A4+2932] net::HttpStreamFactoryImpl::JobController::DoLoop [0x00007FF63E5FB6CB+347] net::HttpStreamFactoryImpl::JobController::RunLoop [0x00007FF63E5F83B8+40] net::ProxyResolutionService::Request::QueryComplete [0x00007FF63E49B0D4+84] net::ProxyResolutionService::SetReady [0x00007FF63E49873B+235] net::ProxyResolutionService::InitializeUsingLastFetchedConfig [0x00007FF63E49899E+446] net::ProxyResolutionService::OnProxyConfigChanged [0x00007FF63E49A2F9+409] network::ProxyConfigServiceMojo::OnProxyConfigUpdated [0x00007FF63F744D4A+186] network::mojom::ProxyConfigClientStubDispatch::Accept [0x00007FF63C54E852+402] mojo::InterfaceEndpointClient::HandleValidatedMessage [0x00007FF63EDFC3C0+656] mojo::FilterChain::Accept [0x00007FF63EE018F7+151] mojo::InterfaceEndpointClient::HandleIncomingMessage [0x00007FF63EDFD37A+122] mojo::internal::MultiplexRouter::ProcessIncomingMessage [0x00007FF63EDF5649+729] mojo::internal::MultiplexRouter::Accept [0x00007FF63EDF515C+348] mojo::FilterChain::Accept [0x00007FF63EE018F7+151] mojo::Connector::ReadSingleMessage [0x00007FF63EDF9B48+440] mojo::Connector::ReadAllAvailableMessages [0x00007FF63EDFA390+128] mojo::Connector::OnHandleReadyInternal [0x00007FF63EDFA1FB+139] mojo::SimpleWatcher::OnHandleReady [0x00007FF63ED4A522+258] base::debug::TaskAnnotator::RunTask [0x00007FF63E2B6788+296] base::internal::IncomingTaskQueue::RunTask [0x00007FF63E2DDF4E+126] base::MessageLoop::RunTask [0x00007FF63E24E079+681] base::MessageLoop::DeferOrRunPendingTask [0x00007FF63E24E4C7+183] base::MessageLoop::DoWork [0x00007FF63E24E70E+542] base::MessagePumpForIO::DoRunLoop [0x00007FF63E294215+165] base::MessagePumpWin::Run [0x00007FF63E2932F8+104] base::MessageLoop::Run [0x00007FF63E24D8D9+201] base::RunLoop::Run [0x00007FF63E22296C+252] base::Thread::Run [0x00007FF63E269B98+200] content::BrowserThreadImpl::IOThreadRun [0x00007FF63CD23477+39] content::BrowserThreadImpl::Run [0x00007FF63CD2361E+302] base::Thread::ThreadMain [0x00007FF63E26A08A+730] base::PlatformThread::GetCurrentThreadPriority [0x00007FF63E24201C+572] BaseThreadInitThunk [0x00007FFAC5E18364+20] RtlUserThreadStart [0x00007FFAC83B5E91+33]
,
Mar 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/788a292e3ec841a0ab78a2186af38cdc980ced8a commit 788a292e3ec841a0ab78a2186af38cdc980ced8a Author: Vadym Doroshenko <dvadym@chromium.org> Date: Mon Mar 19 14:20:20 2018 Disabling flaky test PolicyPrefsTest.PolicyToPrefsMapping on Windows. TBR=emaxx@chromium.org TBR=pastarmovj@chromium.org Bug: 822975 Change-Id: I03062d8eb1c8c3221cf07eb08587e4d9c97ed5e2 Reviewed-on: https://chromium-review.googlesource.com/968507 Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Cr-Commit-Position: refs/heads/master@{#544023} [modify] https://crrev.com/788a292e3ec841a0ab78a2186af38cdc980ced8a/chrome/browser/policy/policy_prefs_browsertest.cc
,
Mar 21 2018
Hi Ramin, can you please prioritize taking a stab at that one? This test is a critical piece of our policy verification infrastructure. It is leaving a big gap on Windows testing currently by not having this test.
,
Mar 28 2018
msramek@: Could you please take a look? Ramin is OOO for the next few days. I'd be nice to fix "PolicyPrefsTest.PolicyToPrefsMapping" ASAP, because, as noted in comment 4, it verifies one of the core pieces in the admin policy subsystem. Do you think that commit e91bd630d3038038491462bb28f63d701850c430 can be related?
,
Mar 28 2018
It doesn't look related. It basically removes a declaration that is not used anywhere anymore. The stack trace shows "MutableNetworkTrafficAnnotationTag". Most network traffic annotations are immutable, but there are a few instances that had to be mutable for one reason or another. The DCHECK(is_valid()) means that there's an annotation with the value TRAFFIC_ANNOTATION_UNINITIALIZED. So there is some mishandling of a mutable annotation somewhere in the policy code.
,
Mar 28 2018
Actually, we're not looking for a mutable annotation necessarily. The mutable annotation in the stack trace is only there because we need to convert to mutable when passing it over mojo. We're simply looking for an unitialized annotation.
,
Mar 28 2018
Well, it doesn't look like this happened in enterprise code. I think you're just unlucky that that the test is exercising some code path which triggers that. The policy is a red herring. Based on the time when this started flaking and a bit of digging around the stack trace, my guess is that there's a mistake in one of these two CLs: https://chromium.googlesource.com/chromium/src/+/ca8d525d49c7f122c84ec4531bdc6ef2d8aae363 https://chromium.googlesource.com/chromium/src/+/921731ea1b3df56cee22f7a9268505cae56b7a90 Annotations are attached to objects and passed around, there's probably some forgotten assignment.
,
Mar 28 2018
I don't see it yet. +mmenke@, +xunjieli@ in case they remember those CLs and have a hunch. If we want to be safe, we could just revert (starting with e91bd630d3038038491462bb28f63d701850c430 which you mentioned, as that landed after them), but a lot of files are affected, so it's likely that this won't revert cleanly. Alternatively, I could disable the validation and let Ramin re-enable it when he returns and can debug. The worst thing that can happen is that people will break some annotations, or add invalid ones, while the validation is disabled. But that's probably acceptable for a short amount of time.
,
Mar 28 2018
I'm fine with disabling the validation - another test ran into this, too (See issue 823077 ), and disabling the DCHECK would fix that one, too, until rhalavati gets back.
,
Mar 28 2018
Thanks Martin for taking look at this quickly! I think your proposal in comment 9 about the validation disabling makes sense, as this allows to re-enable our test immediately and as you estimate the risk for the annotations code as not very high.
,
Mar 28 2018
https://chromium-review.googlesource.com/c/chromium/src/+/984355 will disable the validation once it lands. After that, Maksim, go ahead and re-enable the tests. I'll follow up with Ramin once he returns.
,
Mar 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8255d029d7226bf51208d894e96478ec505d2a53 commit 8255d029d7226bf51208d894e96478ec505d2a53 Author: Maksim Ivanov <emaxx@chromium.org> Date: Thu Mar 29 22:42:38 2018 Revert "Disabling flaky test PolicyPrefsTest.PolicyToPrefsMapping on Windows." This reverts commit 788a292e3ec841a0ab78a2186af38cdc980ced8a. Reason for revert: Re-enabling the PolicyPrefsTest.PolicyToPrefsMapping test, as the flaky DCHECK has been disabled in https://crbug.com/826744 . Original change's description: > Disabling flaky test PolicyPrefsTest.PolicyToPrefsMapping on Windows. > > TBR=emaxx@chromium.org > TBR=pastarmovj@chromium.org > > Bug: 822975 > Change-Id: I03062d8eb1c8c3221cf07eb08587e4d9c97ed5e2 > Reviewed-on: https://chromium-review.googlesource.com/968507 > Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> > Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> > Cr-Commit-Position: refs/heads/master@{#544023} TBR=pastarmovj@chromium.org,emaxx@chromium.org,dvadym@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 822975 Change-Id: I5721a14572d7f7a5b019fd5508ced218f19790a5 Reviewed-on: https://chromium-review.googlesource.com/986097 Reviewed-by: Maksim Ivanov <emaxx@chromium.org> Commit-Queue: Maksim Ivanov <emaxx@chromium.org> Cr-Commit-Position: refs/heads/master@{#546994} [modify] https://crrev.com/8255d029d7226bf51208d894e96478ec505d2a53/chrome/browser/policy/policy_prefs_browsertest.cc
,
Apr 4 2018
Hi, I am back. I take it emaxx@.
,
Apr 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4196b8354cd1bf4aed168dfc0f21074893059303 commit 4196b8354cd1bf4aed168dfc0f21074893059303 Author: Ramin Halavati <rhalavati@chromium.org> Date: Mon Apr 09 08:55:34 2018 Fix ProxyResolutionService annotation bug and reactivate tests. Network traffic annotation in ProxyResolutionService was wrongly set in cases where proxy info was fetched through completely synchronous method, causing flaky test failures. The bug is fixed and tests are re-enabled. Bug: 822975 Bug: 826744 Bug: 823077 Change-Id: Ic63301ff053fc9160b1a96372af1ee289342396c Reviewed-on: https://chromium-review.googlesource.com/997656 Reviewed-by: Eric Roman <eroman@chromium.org> Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Cr-Commit-Position: refs/heads/master@{#549130} [modify] https://crrev.com/4196b8354cd1bf4aed168dfc0f21074893059303/chrome/browser/net/network_context_configuration_browsertest.cc [modify] https://crrev.com/4196b8354cd1bf4aed168dfc0f21074893059303/net/proxy_resolution/proxy_resolution_service.cc [modify] https://crrev.com/4196b8354cd1bf4aed168dfc0f21074893059303/net/traffic_annotation/network_traffic_annotation.h
,
Apr 10 2018
There seems to be no more errors. Marking it fixed. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dvadym@chromium.org
, Mar 19 2018