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

Issue 913400 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 842272



Sign in to add a comment

Regression: Browser crash is seen when switching to other user from one user after enabling any flag

Project Member Reported by kebalaji@chromium.org, Dec 10

Issue description

Chrome Version: 73.0.3635.0/11377.0.0 dev channel Kip, Reks, Daisy
OS: Chrome OS

Pre-Condition: Have 2 user accounts

What steps will reproduce the problem?
(1)Sign-in to one user(User 1)>> Click on Ubertray and Sign-in to another user(User 2)
(2)Enable any flag(Eg: #use-google-local-ntp) in User 1 and restart the browser and switch to User 2 >> Observe Browser Crash 

Actual: Browser crash is seen when switching to other user from one user after enabling any flag
Expected: No such issue should be seen 

This is a Regression issue as same is working fine in 72.0.3626.8/11316.6.0 dev 

Attaching screencasts for reference..

Crash IDS: 11899d2a0c850372
           288da3edc1776dcd
           e2c4bfdd07f74c77
           ee3e4ec18af6b71e

Stack Trace:
------------
Thread 0 (id: 0x40a) CRASHED [SIGSEGV /SEGV_MAPERR @ 0xffffcb103e8a8203 ] MAGIC SIGNATURE THREAD
Stack Quality100%Show frame trust levels
0x00005988901a6190	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/observer_list_internal.h:30 )	(anonymous namespace)::TimezoneSettingsBaseImpl::RemoveObserver(chromeos::system::TimezoneSettings::Observer*)
0x0000598895e9dd57	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc:86 )	chromeos::multidevice_setup::AccountStatusChangeDelegateNotifierImpl::~AccountStatusChangeDelegateNotifierImpl()
0x0000598895e9ddfd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chromeos/services/multidevice_setup/account_status_change_delegate_notifier_impl.cc:84 )	<name omitted>
0x0000598895ea90e0	(chrome -memory:2321 )	chromeos::multidevice_setup::MultiDeviceSetupImpl::~MultiDeviceSetupImpl()
0x0000598895ea92cd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chromeos/services/multidevice_setup/multidevice_setup_impl.cc:156 )	<name omitted>
0x0000598895ea600f	(chrome -memory:2321 )	chromeos::multidevice_setup::MultiDeviceSetupInitializer::~MultiDeviceSetupInitializer()
0x0000598895ea60fd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chromeos/services/multidevice_setup/multidevice_setup_initializer.cc:104 )	<name omitted>
0x0000598895e9ce7d	(chrome -memory:2321 )	chromeos::multidevice_setup::MultiDeviceSetupService::~MultiDeviceSetupService()
0x0000598895e9cecd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chromeos/services/multidevice_setup/multidevice_setup_service.cc:61 )	chromeos::multidevice_setup::MultiDeviceSetupService::~MultiDeviceSetupService()
0x0000598890ad6cdc	(chrome -memory:2321 )	std::__1::__tree<std::__1::__value_type<service_manager::Service*, std::__1::unique_ptr<service_manager::Service, std::__1::default_delete<service_manager::Service> > >, std::__1::__map_value_compare<service_manager::Service*, std::__1::__value_type<service_manager::Service*, std::__1::unique_ptr<service_manager::Service, std::__1::default_delete<service_manager::Service> > >, std::__1::less<service_manager::Service*>, true>, std::__1::allocator<std::__1::__value_type<service_manager::Service*, std::__1::unique_ptr<service_manager::Service, std::__1::default_delete<service_manager::Service> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<service_manager::Service*, std::__1::unique_ptr<service_manager::Service, std::__1::default_delete<service_manager::Service> > >, void*>*)
0x0000598890ad6a4c	(chrome -__tree:1814 )	content::(anonymous namespace)::BrowserContextServiceManagerConnectionHolder::~BrowserContextServiceManagerConnectionHolder()
0x0000598892d2be3c	(chrome -memory:2321 )	std::__1::__tree<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, void*>*)
0x0000598892d2bd15	(chrome -__tree:1814 )	<name omitted>
0x0000598892955fce	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/profiles/profile.cc:87 )	ProfileImpl::~ProfileImpl()
0x00005988929560cd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/profiles/profile_impl.cc:739 )	ProfileImpl::~ProfileImpl()
0x0000598892b83742	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/profiles/profile_destroyer.cc:98 )	ProfileDestroyer::DestroyProfileWhenAppropriate(Profile*)
0x0000598892974e47	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/profiles/profile_manager.cc:1766 )	std::__1::__tree<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::__map_value_compare<base::FilePath, std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::less<base::FilePath>, true>, std::__1::allocator<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, void*>*)
0x0000598892974e23	(chrome -__tree:1824 )	std::__1::__tree<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::__map_value_compare<base::FilePath, std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, std::__1::less<base::FilePath>, true>, std::__1::allocator<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<base::FilePath, std::__1::unique_ptr<ProfileManager::ProfileInfo, std::__1::default_delete<ProfileManager::ProfileInfo> > >, void*>*)
0x0000598892974da4	(chrome -__tree:1814 )	ProfileManager::~ProfileManager()
0x000059889296b0dd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/profiles/profile_manager.cc:390 )	<name omitted>
0x0000598892ae2ee4	(chrome -memory:2321 )	BrowserProcessImpl::StartTearDown()
0x0000598892870af8	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/chrome_browser_main.cc:1891 )	ChromeBrowserMainParts::PostMainMessageLoopRun()
0x00005988917a939d	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/chromeos/chrome_browser_main_chromeos.cc:1131 )	chromeos::ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun()
0x0000598890adf5d1	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/browser/browser_main_loop.cc:1032 )	content::BrowserMainLoop::ShutdownThreadsAndCleanUp()
0x0000598890ae2ddd	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/browser/browser_main_runner_impl.cc:221 )	content::BrowserMainRunnerImpl::Shutdown()
0x0000598890ad7d20	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/browser/browser_main.cc:49 )	content::BrowserMain(content::MainFunctionParams const&)
0x000059889285eef4	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main_runner_impl.cc:543 )	content::ContentMainRunnerImpl::Run(bool)
0x0000598892867039	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/services/service_manager/embedder/main.cc:470 )	service_manager::Main(service_manager::MainParams const&)
0x000059888fb70c5e	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main.cc:19 )	ChromeMain
0x00007e6750fc0a93	(libc-2.27.so -libc-start.c:308 )	__libc_start_main
0x000059888fb544f9	(chrome + 0x002624f9 )	_start
0x00007ffdf840d177		


Link to the list of Builds:
----------------------------
https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%60anonymous+namespace%5C%27%3A%3ATimezoneSettingsBaseImpl%3A%3ARemoveObserver%27+AND+product_name%3D%27Chrome_ChromeOS%27

Assigning to @khorimoto as  mutiple changed related to chromeos/services/multidevice_setup/ filepath in the regression range

 
ExpectedFlags.mp4
15.0 MB Download
ActualFlags.mp4
13.4 MB View Download
Cc: jordynass@chromium.org hsuregan@chromium.org jlklein@chromium.org nohle@chromium.org jhawkins@chromium.org hansberry@chromium.org
Components: UI>Multidevice
It looks like we're attempting to call OobeCompletionTracker::RemoveObserver() after the OobeCompletionTracker instance has been deleted.

When the flag is flipped, the process shuts down, leading to this crash. That being said, I'm not quite sure what the fix should be yet. I'll dig into this more.
Blocking: 842272
Cc: roc...@chromium.org
I saw this crash everytime on signing out. My chrome is at r615175. For me, it repros with a single user. Just sign-in then sign-out. Chrome crashes during shutdown. We should fix this asap.

The analysis in #1 looks correct. The MultiDeviceSetupService mojo service is destructing after the KeyService it depends on. For this particular crash, we may fix by adding a OnOobeCompletionTrackerDestroying to OobeCompletionTracker::Observer and clear relevant references.

+rockot@ since this could be a general per-profile mojo service question. The service is owned by a BrowserContextServiceManagerConnectionHolder as part of user data of a BrowserContext. By the time of the services destruction, we are in SupportUserData destruction and BrowserContext has already finished destruction. If a service has any deps on BrowserContext, we would get into a crash similar to this one. What would be a proper longer term solution? 
Cc: sdefresne@chromium.org msarda@chromium.org blundell@chromium.org
+1 to xiyuan@'s analysis. Ideally, we would be in a completely Mojo-ized world where none of the Mojo services need to depend on any KeyedServices, but we're a long way out from that at this time. There are a few KeyedServices that the multi-device Mojo services depend on, and I'm not sure what the correct general solution is for this.

IdentityService has rolled out its own special shutdown callback (see [1]), but the comments for that function specifically state that it's a custom solution and is not meant for general use. I've CC'ed the IdentityService OWNERS on this bug - perhaps we could figure out how to generalize this for all Mojo services which depend on KeyedServices?

This might also be related to issue 841393 as well.

[1] https://cs.chromium.org/chromium/src/services/identity/identity_service.cc?q=RegisterOnShutdownCallback
I would not conflate this with issue 841393.

I think a simple solution here is to tear down BrowserContextServiceManagerConnectionHolder in BrowserContext::NotifyWillBeDestroyed, guaranteeing that all per-context services are torn down before any other UserData, which should include all KeyedServices.
Cc: khorimoto@chromium.org
Owner: rockot@google.com
I'll go ahead and do that unless you really want to, khorimoto@ :)
Sounds good to me - thanks Ken! Let me know if you need any help from my end.
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 12

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

commit ad3c758a491a0f08788acba07a859e66c42684b8
Author: Ken Rockot <rockot@google.com>
Date: Wed Dec 12 17:04:14 2018

Destroy per-profile Service instances earlier

This tears down all running per-profile Service instances for a specific
BrowserContext within NotifyWillBeDestroyed rather than waiting until
BrowserContext destruction, ensuring that it's safe for Service
instances to refer to other BrowserContext state (e.g. keyed services)
all the way up until their destruction.

Bug:  913400 
Change-Id: I75321476aebd954d1e5768dc2b5d4d81784a55b2
Reviewed-on: https://chromium-review.googlesource.com/c/1372318
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#615928}
[modify] https://crrev.com/ad3c758a491a0f08788acba07a859e66c42684b8/content/browser/browser_context.cc

Status: Fixed (was: Assigned)

Sign in to add a comment