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

Issue 134550 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Crash involving unrecoverable error and "slow" data type (with isolated model association)

Project Member Reported by kareng@google.com, Jun 25 2012

Issue description

Product: Chrome
Stack Signature: -3E4F3B5
New Signature Label: browser_sync::DataTypeManagerImpl::Restart(sync_api::ConfigureReason,browser_sync::BackendDataTypeCo...
New Signature Hash: 185b0373_3fa6b0de_71882040_83ebc061_8f83f68e

Report link: http://go/crash/reportdetail?reportid=5be0288003b78c41

Meta information:
Product Name: Chrome
Product Version: 21.0.1180.4
Report ID: 5be0288003b78c41
Report Time: 2012/06/24 12:30:10, Sun
Uptime: 1530 sec
Cumulative Uptime: 0 sec
OS Name: Windows NT
OS Version: 6.1.7601 Service Pack 1
CPU Architecture: x86
CPU Info: AuthenticAMD family 16 model 6 stepping 2

Thread 0 *CRASHED* ( EXCEPTION_ACCESS_VIOLATION_EXEC @ 0x0000000f )

0x0000000f			
0x5efc5b76	 [chrome.dll]	 - data_type_manager_impl.cc:100	browser_sync::DataTypeManagerImpl::Restart(sync_api::ConfigureReason,browser_sync::BackendDataTypeConfigurer::NigoriState)
0x5efc5c7c	 [chrome.dll]	 - data_type_manager_impl.cc:252	browser_sync::DataTypeManagerImpl::OnTypesLoaded()
0x5f061edf	 [chrome.dll]	 - model_association_manager.cc:425	browser_sync::ModelAssociationManager::ModelLoadCallback(syncable::ModelType,SyncError)
0x5f061ce4	 [chrome.dll]	 - bind_internal.h:938	base::internal::InvokeHelper<1,void,base::internal::RunnableAdapter<void ( browser_sync::ModelAssociationManager::*)(syncable::ModelType,SyncError)>,void (base::WeakPtr<browser_sync::ModelAssociationManager> const &,syncable::ModelType const &,SyncError const &)>::MakeItSo(base::internal::RunnableAdapter<void ( browser_sync::ModelAssociationManager::*)(syncable::ModelType,SyncError)>,base::WeakPtr<browser_sync::ModelAssociationManager> const &,syncable::ModelType const &,SyncError const &)
0x5f061cff	 [chrome.dll]	 - bind_internal.h:1317	base::internal::Invoker<1,base::internal::BindState<base::internal::RunnableAdapter<void ( browser_sync::ModelAssociationManager::*)(syncable::ModelType,SyncError)>,void (browser_sync::ModelAssociationManager *,syncable::ModelType,SyncError),void (base::WeakPtr<browser_sync::ModelAssociationManager>)>,void (browser_sync::ModelAssociationManager *,syncable::ModelType,SyncError)>::Run(base::internal::BindStateBase *,syncable::ModelType const &,SyncError const &)
0x5f063735	 [chrome.dll]	 - new_non_frontend_data_type_controller.cc:176	browser_sync::NewNonFrontendDataTypeController::AbortModelStarting()
0x5f063b1a	 [chrome.dll]	 - new_non_frontend_data_type_controller.cc:103	browser_sync::NewNonFrontendDataTypeController::Stop()
0x5f062e43	 [chrome.dll]	 - model_association_manager.cc:122	browser_sync::ModelAssociationManager::Initialize(browser_sync::EnumSet<syncable::ModelType,2,16>)
0x5efc5b76	 [chrome.dll]	 - data_type_manager_impl.cc:100	browser_sync::DataTypeManagerImpl::Restart(sync_api::ConfigureReason,browser_sync::BackendDataTypeConfigurer::NigoriState)
0x5efc5624	 [chrome.dll]	 - data_type_manager_impl.cc:93	browser_sync::DataTypeManagerImpl::ConfigureImpl(browser_sync::EnumSet<syncable::ModelType,2,16>,sync_api::ConfigureReason,browser_sync::BackendDataTypeConfigurer::NigoriState)
0x5efc56ea	 [chrome.dll]	 - data_type_manager_impl.cc:51	browser_sync::DataTypeManagerImpl::Configure(browser_sync::EnumSet<syncable::ModelType,2,16>,sync_api::ConfigureReason)
0x5ee558e8	 [chrome.dll]	 - profile_sync_service.cc:1255	ProfileSyncService::ConfigureDataTypeManager()
0x5ee559c2	 [chrome.dll]	 - profile_sync_service.cc:1664	ProfileSyncService::ReconfigureDatatypeManager()
0x5ee5494a	 [chrome.dll]	 - profile_sync_service.cc:1025	ProfileSyncService::SetSetupInProgress(bool)
0x5ef29d1c	 [chrome.dll]	 - sync_setup_handler.cc:865	SyncSetupHandler::CloseSyncSetup()
0x5ef2a985	 [chrome.dll]	 - sync_setup_handler.cc:566	SyncSetupHandler::OnDidClosePage(base::ListValue const *)
0x5e1ffec5	 [chrome.dll]	 - bind_internal.h:1225	base::internal::Invoker<1,base::internal::BindState<base::internal::RunnableAdapter<void ( SuggestionsInternalsUIHandler::*)(base::ListValue const *)>,void (SuggestionsInternalsUIHandler *,base::ListValue const *),void (base::internal::UnretainedWrapper<SuggestionsInternalsUIHandler>)>,void (SuggestionsInternalsUIHandler *,base::ListValue const *)>::Run(base::internal::BindStateBase *,base::ListValue const * const &)
0x5e529ebc	 [chrome.dll]	 - web_ui_impl.cc:257	WebUIImpl::ProcessWebUIMessage(GURL const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::ListValue const &)
0x5ee70138	 [chrome.dll]	 - uber_ui.cc:180	UberUI::OverrideHandleWebUIMessage(GURL const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::ListValue const &)
0x5e529e95	 [chrome.dll]	 - web_ui_impl.cc:249	WebUIImpl::ProcessWebUIMessage(GURL const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::ListValue const &)
0x5e529e76	 [chrome.dll]	 - web_ui_impl.cc:98	WebUIImpl::OnWebUISend(GURL const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::ListValue const &)
0x5e529c84	 [chrome.dll]	 - view_messages.h:1733	ViewHostMsg_WebUISend::Dispatch<WebContentsImpl,WebContentsImpl,void ( WebContentsImpl::*)(GURL const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::ListValue const &)>(IPC::Message const *,WebContentsImpl *,WebContentsImpl *,void ( WebContentsImpl::*)(GURL const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,base::ListValue const &))
0x5e515fab	 [chrome.dll]	 - web_ui_impl.cc:76	WebUIImpl::OnMessageReceived(IPC::Message const &)
0x5e51584f	 [chrome.dll]	 - web_contents_impl.cc:603	WebContentsImpl::OnMessageReceived(IPC::Message const &)
0x5e51437e	 [chrome.dll]	 - render_view_host_impl.cc:834	content::RenderViewHostImpl::OnMessageReceived(IPC::Message const &)
0x5e5142c5	 [chrome.dll]	 - render_process_host_impl.cc:939	content::RenderProcessHostImpl::OnMessageReceived(IPC::Message const &)
0x5e09f172	 [chrome.dll]	 - ipc_channel_proxy.cc:250	IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const &)
0x5e09c391	 [chrome.dll]	 - bind_internal.h:1254	base::internal::Invoker<2,base::internal::BindState<base::internal::RunnableAdapter<void ( CrxInstaller::*)(CrxInstallerError const &)>,void (CrxInstaller *,CrxInstallerError const &),void (CrxInstaller *,CrxInstallerError)>,void (CrxInstaller *,CrxInstallerError const &)>::Run(base::internal::BindStateBase *)
0x5e09d6af	 [chrome.dll]	 - message_loop.cc:465	MessageLoop::RunTask(base::PendingTask const &)
0x5e09bbfe	 [chrome.dll]	 - message_loop.cc:654	MessageLoop::DoWork()
0x5e2439e9	 [chrome.dll]	 - message_pump_win.cc:239	base::MessagePumpForUI::DoRunLoop()
0x5e09b78f	 [chrome.dll]	 - message_loop.cc:419	MessageLoop::RunInternal()
0x5e4fd3d4	 [chrome.dll]	 - message_loop.cc:770	MessageLoopForUI::RunWithDispatcher(base::MessagePumpDispatcher *)
0x5e4fced4	 [chrome.dll]	 - chrome_browser_main.cc:1922	ChromeBrowserMainParts::MainMessageLoopRun(int *)
0x5e4fccac	 [chrome.dll]	 - browser_main_loop.cc:440	content::BrowserMainLoop::RunMainMessageLoopParts()
0x5e4fcbaf	 [chrome.dll]	 - browser_main_runner.cc:98	`anonymous namespace'::BrowserMainRunnerImpl::Run()
0x5e1216f8	 [chrome.dll]	 - browser_main.cc:21	BrowserMain(content::MainFunctionParams const &)
0x5e097908	 [chrome.dll]	 - content_main_runner.cc:371	content::RunNamedProcessTypeMain(std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,content::MainFunctionParams const &,content::ContentMainDelegate *)
0x5e09788f	 [chrome.dll]	 - content_main_runner.cc:626	content::ContentMainRunnerImpl::Run()
0x5e08a2b7	 [chrome.dll]	 - content_main.cc:35	content::ContentMain(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *,content::ContentMainDelegate *)
0x5e08a243	 [chrome.dll]	 - chrome_main.cc:28	ChromeMain
0x012d63aa	 [chrome.exe]	 - client_util.cc:423	MainDllLoader::Launch(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *)
0x012d55a6	 [chrome.exe]	 - chrome_exe_main_win.cc:31	RunChrome(HINSTANCE__ *)
0x012d5611	 [chrome.exe]	 - chrome_exe_main_win.cc:47	wWinMain
0x0132d972	 [chrome.exe]	 - crt0.c:275	__tmainCRTStartup
0x76db3399	 [kernel32.dll]	 + 0x00013399]	BaseThreadInitThunk
0x77529ef1	 [ntdll.dll]	 + 0x00039ef1]	__RtlUserThreadStart
0x77529ec4	 [ntdll.dll]	 + 0x00039ec4]	_RtlUserThreadStart

top crash for m21 pls take a look :)
 

Comment 1 by tim@chromium.org, Jun 25 2012

Cc: lipalani@google.com lipalani@chromium.org

Comment 2 by tim@chromium.org, Jun 25 2012

Hmm... looks like a late callback invocation (objects are destroyed, etc). Looking..

Comment 3 by tim@chromium.org, Jun 25 2012

Cc: nyquist@chromium.org atwilson@chromium.org
Labels: Feature-Sync
Possibly related to https://chromiumcodereview.appspot.com/10534075

Following up w/ nyquist.

Comment 4 by tim@chromium.org, Jun 26 2012

I found a few instances that don't seem to be coming from the WebUI callsite and the path in the CL linked to above - http://crash/reportdetail?reportid=db03b6d8e472c491

That said, I see no traces of a crash with OnTypesLoaded happening before last Thursday, which is suspicious.

http://crash.corp.google.com/reportdetail?reportid=39c56fb255944a82 is crashing on a null deref of a DataTypeController in ModelAssociationManager::Initialize, which is on the stack twice, so I suspect a reentrancy issue where we're clearing controllers and not expecting to wind back up in here.  Don't understand this yet.

Comment 5 by tim@chromium.org, Jun 26 2012

It looks like this was crashing before nyquist@'s patch, although that patch seems to have made it much more likely.

http://crash.corp.google.com/reportdetail?reportid=46252713fcb92d06 occurred last Thursday but on a build that was pushed on 6/8. 

Comment 6 by tim@chromium.org, Jun 26 2012

Summary: Crash involving unrecoverable error and "slow" data type (with isolated model association)
What I've pieced together so far:

1. ModelAssociation completes for some but not all types (some take longer than the allotted deadline).
2. a datatype attempts to disable itself due to failure. possibly the same type, not sure.
3. a reconfiguration (without disabled datatype) is initiated as a result, and since the DataTypeManager is CONFIGURED, we go into Restart.
4. The ModelAssociationManager notices at least one type that hasn't finished associating (seems to be a nonfrontend type consistently). It tries to Stop() it.
5. NNFDTC::AbortModelStarting invokes the load callback with 'ABORTED'
6. DTM::OnTypesLoaded calls Restart, again pointing to the fact that the DTM was CONFIGURED (otherwise it bails out early).

However, something (seemingly the DTM) is actually garbage at this point, as if something caused it to be destroyed between steps 3 and 6.  I don't know what this could be yet.  I thought perhaps the model_load_callback in the NNFDTC was bogus / unset (we don't seem to safety check it anywhere), but then how would it have actually wound up in DataTypeManager?

Comment 7 by tim@chromium.org, Jun 26 2012

Okay, after finding a report with a bit more stack info 
(http://crash.corp.google.com/reportdetail?reportid=c4e58d18881d6ed1)

it looks like the crash happens when we try to loop over pending_model_load_ in the second (reentrant) call to MAM::Initialize, suggesting the DTC is dangling / garbage.  

It looks like one bug is that we set_state(STOPPING) in NFNDTC::Stop() before calling AbortModelLoad, which makes the reentrant Initialize call, which will try to loop over the list of DTCs again.  Theoretically it could reach the end of the list and clear it before the first call finishes iterating.

Comment 8 by tim@chromium.org, Jun 26 2012

Another issue seems to be that MAM::Initialize expects to be in state IDLE (given the DCHECK), yet the reentrancy clearly shows it is in INITIALIZED_TO_CONFIGURE.

But the most dangerous looking thing I've seen so far is that we're calling vector.erase while iterating over the pending_model_load from *multiple* possible places.  This is fraught with peril and I bet is the core problem here, although there seems to be a higher level breakdown between components given invariant violations.

Comment 9 by tim@chromium.org, Jun 26 2012

From looking at lines 93 and 100 in  https://chromiumcodereview.appspot.com/10387144/diff/24002/chrome/browser/sync/glue/data_type_manager_impl.cc 

I'm tempted to say a (partial?) fix is to move Initialize out of Restart.  Have to think this through some more and hopefully get Lingesh's input!

Comment 10 by tim@chromium.org, Jun 26 2012

On second though, OnTypesLoaded > Restart may be the swerve that sends us careening off the cliff.

Problem being, even though the DataTypeManager is CONFIGURED, we're definitely *not* in a happy state.  We've entered Restart.

So options I'll look at tomorrow in addition to last comment are moving the MAM code in Restart to after a point where we've transitioned state_, and/or possibly making MAM::Initialize smart enough to bail out in this case.

Comment 11 by tim@chromium.org, Jun 26 2012

Ok, Lingesh helped sort this out, it does look like the OnTypesLoaded call is the wrong turn.  The oversight in the code seems to be in ModelLoadCallback, we should not call OnTypesLoaded if we're in any state other than IDLE, because it implies some sort of error case.

I will also change the DCHECK_EQ(state_, IDLE) in ModelAssociationManager::Initialize to be a CHECK for a dev channel release, since we should *not* be reentering there, and I want to make sure there aren't other bugs lurking.

Comment 12 by Deleted ...@, Jun 27 2012

                                  racker 
sistema operacional rackeado racker impresa racker hyps pesquisem mas para entrar no site presiza de senha e a senha é:368848771559548porkejlnvoirneikiaidg4752545 o imail é: hypslugrw@hyps.com visitem kkkkkkkkk quem leu foi um otario eu escrevi tudo atoa
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 27 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=144583

------------------------------------------------------------------------
r144583 | tim@chromium.org | Wed Jun 27 15:19:29 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/glue/model_association_manager.cc?r1=144583&r2=144582&pathrev=144583
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/glue/fake_data_type_controller.cc?r1=144583&r2=144582&pathrev=144583
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/glue/model_association_manager_unittest.cc?r1=144583&r2=144582&pathrev=144583
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/glue/model_association_manager.h?r1=144583&r2=144582&pathrev=144583

sync: fix reentrancy crash in ModelAssociationManager

Don't call OnTypesLoaded unless we're IDLE, since we could be in the middle of some 
other process.  

Add a CHECK to ensure we don't re-enter ModelAssociationManager::Initialize.

BUG= 134550 
TEST=trigger unrecoverable error with data type while data type configuration is in progress, don't crash.


Review URL: https://chromiumcodereview.appspot.com/10687002
------------------------------------------------------------------------

Comment 14 by tim@chromium.org, Jun 28 2012

Labels: Merge-Requested

Comment 15 by tim@chromium.org, Jun 28 2012

Sorry if I jumped the gun with the merge request. Old habits die hard :|

Comment 16 by tim@chromium.org, Jun 29 2012

Some data for TPMs: I haven't seen any new issues arise due to this, although the original crash wasn't happening much on canary.  I think we should merge to 21 as soon as is feasible.  Note I'll be out for two weeks starting Monday, so may not be around to do the actual drovering (unless tomorrow works!).

Comment 17 by kareng@google.com, Jun 29 2012

ok we agreed we'll hold off until july 9.

Comment 18 by kareng@google.com, Jul 12 2012

Labels: -Merge-Requested Merge-Approved
we see this has gone down significantly in m22 and pretty high in m21. so we'll take it.
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 12 2012

Labels: -Merge-Approved merge-merged-1180
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=146422

------------------------------------------------------------------------
r146422 | zea@chromium.org | Thu Jul 12 13:01:23 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager_unittest.cc?r1=146422&r2=146421&pathrev=146422
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager.h?r1=146422&r2=146421&pathrev=146422
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager.cc?r1=146422&r2=146421&pathrev=146422
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/fake_data_type_controller.cc?r1=146422&r2=146421&pathrev=146422

Merge 144583 - sync: fix reentrancy crash in ModelAssociationManager

Don't call OnTypesLoaded unless we're IDLE, since we could be in the middle of some 
other process.  

Add a CHECK to ensure we don't re-enter ModelAssociationManager::Initialize.

BUG= 134550 
TEST=trigger unrecoverable error with data type while data type configuration is in progress, don't crash.


Review URL: https://chromiumcodereview.appspot.com/10687002

TBR=tim@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10689178
------------------------------------------------------------------------
Project Member

Comment 20 by bugdroid1@chromium.org, Jul 12 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=146437

------------------------------------------------------------------------
r146437 | zea@chromium.org | Thu Jul 12 13:59:41 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager_unittest.cc?r1=146437&r2=146436&pathrev=146437
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager.h?r1=146437&r2=146436&pathrev=146437
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager.cc?r1=146437&r2=146436&pathrev=146437
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/fake_data_type_controller.cc?r1=146437&r2=146436&pathrev=146437

Revert 146422 - Merge 144583 - sync: fix reentrancy crash in ModelAssociationManager

Don't call OnTypesLoaded unless we're IDLE, since we could be in the middle of some 
other process.  

Add a CHECK to ensure we don't re-enter ModelAssociationManager::Initialize.

BUG= 134550 
TEST=trigger unrecoverable error with data type while data type configuration is in progress, don't crash.


Review URL: https://chromiumcodereview.appspot.com/10687002

TBR=tim@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10689178

TBR=zea@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10694168
------------------------------------------------------------------------
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 12 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=146446

------------------------------------------------------------------------
r146446 | zea@chromium.org | Thu Jul 12 14:32:23 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager_unittest.cc?r1=146446&r2=146445&pathrev=146446
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager.h?r1=146446&r2=146445&pathrev=146446
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/model_association_manager.cc?r1=146446&r2=146445&pathrev=146446
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/chrome/browser/sync/glue/fake_data_type_controller.cc?r1=146446&r2=146445&pathrev=146446

Merge 144583 - sync: fix reentrancy crash in ModelAssociationManager

Don't call OnTypesLoaded unless we're IDLE, since we could be in the middle of some 
other process.  

Add a CHECK to ensure we don't re-enter ModelAssociationManager::Initialize.

BUG= 134550 
TEST=trigger unrecoverable error with data type while data type configuration is in progress, don't crash.


Review URL: https://chromiumcodereview.appspot.com/10687002

TBR=tim@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10696207
------------------------------------------------------------------------

Comment 22 by kareng@google.com, Jul 16 2012

Status: Fixed
closing. merged to branch.

Comment 23 by tim@chromium.org, Aug 7 2012

 Issue 131718  has been merged into this issue.
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 25 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-UI -Mstone-21 -Feature-Sync Cr-Services-Sync Cr-UI M-21
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 14 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment