New issue
Advanced search Search tips

Issue 801554 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

CHECK failure on shutdown in KDE proxy config code

Project Member Reported by ricea@chromium.org, Jan 12 2018

Issue description

To reproduce: Run chrome (or browser_tests) under KDE on Linux. Exit the process.

Expected behaviour: Doesn't CHECK fail on shutdown
Actual behaviour:

[120208:120208:0112/222910.742870:FATAL:file_descriptor_watcher_posix.cc(29)] Check failed: sequence_checker_.CalledOnValidSequence(). 
#0 0x0000050a89bc base::debug::StackTrace::StackTrace()
#1 0x0000050c8aec logging::LogMessage::~LogMessage()
#2 0x0000050aee02 base::FileDescriptorWatcher::Controller::~Controller()
#3 0x00000594314e net::(anonymous namespace)::SettingGetterImplKDE::~SettingGetterImplKDE()
#4 0x00000594329e net::(anonymous namespace)::SettingGetterImplKDE::~SettingGetterImplKDE()
#5 0x00000593d62a net::ProxyConfigServiceLinux::Delegate::~Delegate()
#6 0x0000059404b7 net::ProxyConfigServiceLinux::~ProxyConfigServiceLinux()
#7 0x0000059404ee net::ProxyConfigServiceLinux::~ProxyConfigServiceLinux()
#8 0x0000071dd17a ProxyConfigServiceImpl::~ProxyConfigServiceImpl()
#9 0x0000052a9ea8 ProxyConfigMonitor::~ProxyConfigMonitor()
#10 0x0000051ead41 BrowserProcessImpl::PostDestroyThreads()
#11 0x0000051e9bbb ChromeBrowserMainParts::PostDestroyThreads()
#12 0x000003a4aa73 content::BrowserMainLoop::ShutdownThreadsAndCleanUp()
#13 0x000003a4d36d content::BrowserMainRunnerImpl::Shutdown()
#14 0x000003a457d6 content::BrowserMain()
#15 0x0000050877eb content::RunNamedProcessTypeMain()
#16 0x0000050882c6 content::ContentMainRunnerImpl::Run()
#17 0x000006ff0064 service_manager::Main()
#18 0x000005086b31 content::ContentMain()
#19 0x0000057030ae content::BrowserTestBase::SetUp()
#20 0x000005182e3b InProcessBrowserTest::SetUp()
#21 0x0000030dceb1 testing::Test::Run()
#22 0x0000030dd9e0 testing::TestInfo::Run()
#23 0x0000030ddec7 testing::TestCase::Run()
#24 0x0000030e44f7 testing::internal::UnitTestImpl::RunAllTests()
#25 0x0000030e4177 testing::UnitTest::Run()
#26 0x000005198982 base::TestSuite::Run()
#27 0x00000509b142 ChromeTestSuiteRunner::RunTestSuite()
#28 0x00000573c609 content::LaunchTests()
#29 0x00000509b511 LaunchChromeTests()
#30 0x00000509b09e main
#31 0x7f560a0aef45 __libc_start_main
#32 0x000001f5d02a _start

git bisect indicates culprit:

d0d0d0508a541932ce4e43dfdeec1eb6dacf9edf is the first bad commit
commit d0d0d0508a541932ce4e43dfdeec1eb6dacf9edf
Author: Matt Menke <mmenke@chromium.org>
Date:   Fri Dec 1 19:00:50 2017 +0000

    Send proxy configuration over Mojo pipe in Chrome.
    
    Do this by moving ProxyConfigService creation and ownership
    from the legacy IOThread/ProfileIOData classes on the IO thread
    to a new ProxyConfigMonitor class, which lives on the UI thread
    and is owned by the new SystemNetworkContextManager and
    ProfileNetworkContextService classes.
    
    Chrome's ProxyConfigService now sends messages to the network
    stack over a Mojo pipe from the UI thread over to the IO thread
    (or the network process) instead of plugging directly in to the
    ProxyService on the IO thread.
    
    This effectively adds support for non-PAC proxy configurations
    when the network service is enabled.
    
    This CL also makes NetworkContext default to using direct
    connections, instead of the system proxy configuration, both
    to ensure a NetworkContext can always successfully be created,
    and because it's more sandbox friendly.  Included in this CL
    because without it, tests fail on Linux and ChromeOS when the
    network service is enabled, because the legacy in-process
    URLRequestContextBuilder can't create a ProxyConfigService on
    those platforms.
    
    Bug:  754007 
    Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.mac:ios-simulator-cronet
    Change-Id: I537514ae8bf3726b3215bcf9b9b27f2d8ae2a951
    Reviewed-on: https://chromium-review.googlesource.com/760538
    Commit-Queue: Matt Menke <mmenke@chromium.org>
    Reviewed-by: Eric Roman <eroman@chromium.org>
    Reviewed-by: Scott Violet <sky@chromium.org>
    Reviewed-by: Tom Sepez <tsepez@chromium.org>
    Reviewed-by: John Abd-El-Malek <jam@chromium.org>
    Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#521003}
 

Comment 1 by mmenke@chromium.org, Jan 12 2018

Status: Started (was: Assigned)
So I guess the issue is that we're now destroying the ProxyConfigService in PostDestroyThreads, so we (unsurprisingly) have trouble closing its file-related stuff off thread.  I guess we can tear it down earlier, since it talks to the network service via mojo only, so it's safe to destroy before the service it's actually talking to.

Comment 2 by ricea@chromium.org, Jan 16 2018

In case anyone else is stuck on this, the way to make the browser_tests work is to run

export -n XDG_CURRENT_DESKTOP DESKTOP_SESSION KDE_FULL_SESSION
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 17 2018

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

commit 02bf09f46014c1c8f8ed0680b7789942a7635180
Author: Matt Menke <mmenke@chromium.org>
Date: Wed Jan 17 21:18:36 2018

Fix KDE crash

SettingGetterImplKDE expects to be able to post a shutdown task on
destruction, but that only works if there's actually a thread to post
tasks to. This CL destroys the SystemNetworkContextManager (Which now
owns SettingGetterImplKDE, and all other proxy-config classes) before
threads are torn down to fix the crash.

Bug:  801554 
Change-Id: I167512fd831e9c01e9eaef95f3293d2cd8d8dbbd
Reviewed-on: https://chromium-review.googlesource.com/864904
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529894}
[modify] https://crrev.com/02bf09f46014c1c8f8ed0680b7789942a7635180/chrome/browser/browser_process_impl.cc

Comment 4 by mmenke@chromium.org, Jan 17 2018

Status: Fixed (was: Started)
This should be fixed.  Please tell me if you still encounter the problem, once you've updated to a version with this patch.

Comment 5 by ricea@chromium.org, Jan 18 2018

Status: Verified (was: Fixed)

Sign in to add a comment