Currently KeyedServiceFactory and friends have a debug assertion for context use-after-destroy (AssertContextWasntDestroyed). But there is a problem - this assertion is located in the virtual method GetBrowserContextToUse() which is almost always overriden by user factories. Because of that, the assertion is almost always ignored! This can create unpleasant situations with invalid service destructions which can lead to memory corruption and more.
You can see the current implementation here:
https://chromium.googlesource.com/chromium/src.git/+/59.0.3041.2/components/keyed_service/content/browser_context_keyed_base_factory.cc#24
https://chromium.googlesource.com/chromium/src.git/+/59.0.3041.2/components/keyed_service/content/browser_context_keyed_service_factory.cc#54
Adding to that, this assertion works only in debug mode and doesn't have an effect in release builds. We had some crashes which were traced down to the invalid service shutdown, and we decided to add DumpWithoutCrashes() for this situation in production.
I've prepared CL with all required improvements and fixes, will upload it soon.
--
And there is one more thing: the virtual method GetBrowserContextToUse() also contains an important DCHECK(CalledOnValidThread()) which is also almost always ignored. When I enabled it for all services, I found few problems in NaCl and Sync service implementations. I will create another issue for that and link it in the CL.
NaCl:
[22036:14544:0314/060338.808:FATAL:keyed_service_base_factory.cc(87)] Check failed: CalledOnValidThread().
Backtrace:
base::debug::StackTrace::StackTrace [0x70473E40+32]
base::debug::StackTrace::StackTrace [0x704732AD+13]
logging::LogMessage::~LogMessage [0x7049DB11+49]
KeyedServiceBaseFactory::AssertContextWasntDestroyed [0x6A4A62CC+90]
BrowserContextKeyedServiceFactory::GetContextToUse [0x69DA3698+14]
KeyedServiceFactory::GetServiceForContext [0x6A4A6858+196]
extensions::ExtensionSystemFactory::GetForBrowserContext [0x02FB3615+23]
extensions::ExtensionSystem::Get [0x01C9D707+27]
NaClBrowserDelegateImpl::GetExtensionInfoMap [0x027A07D3+103]
NaClBrowserDelegateImpl::MapUrlToLocalFilePath [0x027A0740+36]
nacl::NaClHostMessageFilter::LaunchNaClContinuationOnIOThread [0x037EE56A+414]
Sync service:
(1) [11052:14732:0314/060126.726:FATAL:keyed_service_base_factory.cc(87)] Check failed: CalledOnValidThread().
Backtrace:
base::debug::StackTrace::StackTrace [0x70963E40+32]
base::debug::StackTrace::StackTrace [0x709632AD+13]
logging::LogMessage::~LogMessage [0x7098DB11+49]
KeyedServiceBaseFactory::AssertContextWasntDestroyed [0x6C3762CC+90]
BrowserContextKeyedServiceFactory::GetContextToUse [0x6BDA3698+14]
KeyedServiceFactory::GetServiceForContext [0x6C376858+196]
extensions::StorageFrontend::Get [0x00DF60D8+28]
extensions::settings_sync_util::GetSyncableService [0x01D361A5+193]
browser_sync::ChromeSyncClient::GetSyncableServiceForType [0x0148324A+130]
syncer::SharedChangeProcessor::Connect [0x02AA0023+311]
syncer::SharedChangeProcessor::StartAssociation [0x02A9F80A+206]
(2) [10612:11124:0314/060126.867:FATAL:keyed_service_base_factory.cc(87)] Check failed: CalledOnValidThread().
Backtrace:
base::debug::StackTrace::StackTrace [0x70963E40+32]
base::debug::StackTrace::StackTrace [0x709632AD+13]
logging::LogMessage::~LogMessage [0x7098DB11+49]
KeyedServiceBaseFactory::AssertContextWasntDestroyed [0x6C3762CC+90]
BrowserContextKeyedServiceFactory::GetContextToUse [0x6BDA3698+14]
KeyedServiceFactory::GetServiceForContext [0x6C376858+196]
HistoryServiceFactory::GetForProfile [0x014A160A+110]
browser_sync::ChromeSyncClient::GetSyncableServiceForType [0x014832C1+249]
syncer::SharedChangeProcessor::Connect [0x02AA0023+311]
syncer::SharedChangeProcessor::StartAssociation [0x02A9F80A+206]
Comment 1 by se...@yandex-team.ru
, Mar 14 2017