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

Issue 120978 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Startup crash in ExtensionSorting::SyncIfNeeded.

Project Member Reported by sh...@chromium.org, Mar 29 2012

Issue description

http://crash/reportdetail?reportid=992c57f8dd6f8caf

Thread 0 *CRASHED* ( EXC_BREAKPOINT / EXC_I386_BPT @ 0x669230a7 )

0x669230a7	 [Google Chrome Framework	 - debugger_posix.cc:216]	base::debug::BreakDebugger
0x6817440f	 [Google Chrome Framework	 - extension_sorting.cc:508]	ExtensionSorting::SyncIfNeeded
0x68172f24	 [Google Chrome Framework	 - extension_sorting.cc:286]	ExtensionSorting::SetAppLaunchOrdinal
0x681734ba	 [Google Chrome Framework	 - extension_sorting.cc:187]	ExtensionSorting::FixNTPOrdinalCollisions
0x6816b0ed	 [Google Chrome Framework	 - extension_service.cc:1391]	ExtensionService::MergeDataAndStartSyncing
0x66613a67	 [Google Chrome Framework	 - ui_data_type_controller.cc:133]	browser_sync::UIDataTypeController::Associate
0x6661367a	 [Google Chrome Framework	 - ui_data_type_controller.cc:79]	browser_sync::UIDataTypeController::Start
0x665e9933	 [Google Chrome Framework	 - data_type_manager_impl.cc:326]	browser_sync::DataTypeManagerImpl::StartNextType
0x665e9cfb	 [Google Chrome Framework	 - data_type_manager_impl.cc:404]	browser_sync::DataTypeManagerImpl::TypeStartCallback
0x665ea3fd	 [Google Chrome Framework	 - ../base/bind_internal.h:246]	base::internal::Invoker<1, base::internal::BindState<base::internal::RunnableAdapter<void (browser_sync::DataTypeManagerImpl::*)(browser_sync::DataTypeController::StartResult, const SyncError &)>, void (browser_sync::DataTypeManagerImpl *, browser_sync::DataTypeController::StartResult, const SyncError &), void (base::WeakPtr<browser_sync::DataTypeManagerImpl>)>, void (browser_sync::DataTypeManagerImpl *, browser_sync::DataTypeController::StartResult, const SyncError &)>::Run
0x665f1fb7	 [Google Chrome Framework	 - ../base/callback.h:353]	browser_sync::NonFrontendDataTypeController::StartDoneImpl
0x665f10aa	 [Google Chrome Framework	 - new_non_frontend_data_type_controller.cc:174]	browser_sync::NewNonFrontendDataTypeController::StartDoneImpl
0x665f15e7	 [Google Chrome Framework	 - ../base/bind_internal.h:309]	base::internal::Invoker<4, base::internal::BindState<base::internal::RunnableAdapter<void (browser_sync::NewNonFrontendDataTypeController::*)(browser_sync::DataTypeController::StartResult, browser_sync::DataTypeController::State, const SyncError &)>, void (browser_sync::NewNonFrontendDataTypeController *, browser_sync::DataTypeController::StartResult, browser_sync::DataTypeController::State, const SyncError &), void (browser_sync::NewNonFrontendDataTypeController *, browser_sync::DataTypeController::StartResult, browser_sync::DataTypeController::State, SyncError)>, void (browser_sync::NewNonFrontendDataTypeController *, browser_sync::DataTypeController::StartResult, browser_sync::DataTypeController::State, const SyncError &)>::Run
0x6693b83a	 [Google Chrome Framework	 - ../base/callback.h:272]	MessageLoop::RunTask
0x6693ba2b	 [Google Chrome Framework	 - message_loop.cc:470]	MessageLoop::DeferOrRunPendingTask
0x6693bcfb	 [Google Chrome Framework	 - message_loop.cc:660]	MessageLoop::DoWork
0x6691048b	 [Google Chrome Framework	 - message_pump_mac.mm:262]	base::MessagePumpCFRunLoopBase::RunWork

 

Comment 1 by sh...@chromium.org, Mar 29 2012

Cc: andywarr@chromium.org
Labels: -Area-Undefined -Pri-2 Area-UI Pri-1 Stability-Crash Feature-Sync Mstone-19
Owner: akalin@chromium.org
This has started happening recently.  Googler reported it (cc'ed).  I see other crashes starting at 19.0.1078.0.  Crashed a few seconds after startup, but not always reliably (sometimes instead of crashing hard, it seemed to get wedged using CPU and memory).  Was able to launch with --disable-sync, but then we couldn't see login info and the like, so I launched with the wifi disabled and then we could see that sync was enabled, he was logged in, etc.  And it didn't crash.

I tried narrowing down specific --disable-sync-* flags, but found that this set:

disable-sync-app-settings
disable-sync-apps
disable-sync-app-notifications
disable-sync-autofill
disable-sync-autofill-profile
disable-sync-bookmarks
disable-sync-extension-settings
disable-sync-extensions

prevented the crash, while removing the last one had it crashing, but simply adding --disable-sync-extensions did not prevent the crash.  So it seems likely to be some interaction, like maybe --disable-sync-extensions --disable-sync-apps would work or something.

Brought up with wifi disabled, turned off all sync except autofill, and everything seems to work.  I copied aside the profile on Andy's computer, but given the current settings, it is likely that his regular canary profile will be able to repro by turning on sync of everything.

Since this is a startup crash, it seems high-priority, like dev-channel blocker or something.  But I'll leave it to you to mark it so, if that's reasonable.

Comment 2 by sh...@chromium.org, Mar 29 2012

Cc: csharp@chromium.org
+csharp due to http://codereview.chromium.org/9706017/ 

Comment 3 by sh...@chromium.org, Mar 29 2012

Labels: -OS-Mac OS-All
+Windows canary: http://crash/reportdetail?reportid=216c93eea19ff4b3

Comment 4 by zea@chromium.org, Mar 29 2012

Labels: Feature-Extensions

Comment 5 by zea@chromium.org, Mar 29 2012

Owner: csharp@chromium.org
Status: Assigned
Assiging to CSharp since Fred is on vacation and it looks like he introduced that method.

Comment 6 by zea@chromium.org, Mar 29 2012

Labels: ReleaseBlock-Stable
Also marking as release blocker. Looks like there's 26 of these crashes on 19.0.1083.0.

Comment 7 by laforge@google.com, Mar 29 2012

Labels: -ReleaseBlock-Stable ReleaseBlock-Beta
Given that we are talking about a startup crash... up-ing to release block beta since this will most certainly cost us users.

Comment 8 by akalin@chromium.org, Mar 30 2012

Yeah, CSharp is the right owner for this.

It looks like the crash happens if a non-app extension is passed into SetAppLaunchOrdinal.  Looks like some callers may not be making checking is_app() first?

Comment 9 by csharp@chromium.org, Mar 30 2012

Got a patch out for review to fix the issue, https://chromiumcodereview.appspot.com/9956020/

The problem is that there are extension with valid ordinal values in the system so they are loaded into the ordinal mapping. If they then later have a collision with another ordinal we try to change there value and then ExtensionSorting crashes because it's syncing for a non-app. 

I'm fairly certain these are older extension, which were able to get valid indexes in the past, which were then converted to the new ordinal format. When the new ordinals were created there wasn't a way to ensure they were for apps, and later on only apps were assume to have valid ordinals, which led to the crash condition.
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 31 2012

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

------------------------------------------------------------------------
r130040 | csharp@chromium.org | Fri Mar 30 21:13:37 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_sorting.cc?r1=130040&r2=130039&pathrev=130040

Fix Crash in ExtensionSorting::SyncIfNeeded

Replace CHECK with if statement, since it is possible to have old extensions that have ordinal values.


BUG= 120978 
TEST=Old profiles with extensions that have ordinal values don't crash on startup


Review URL: http://codereview.chromium.org/9956020
------------------------------------------------------------------------
Thanks for fixing it! I don't see it anymore in 20.0.1088.0.
Labels: Merge-Requested
I think this was happening often enough per day on canary that not having it since 1087 means it's good to go.  Marking so Anthony considers it for M-19.
Labels: -Merge-Requested Merge-Approved
Status: Fixed
Merged, marking as fixed.
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 2 2012

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

------------------------------------------------------------------------
r130243 | laforge@chromium.org | Mon Apr 02 15:57:17 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1084/src/chrome/browser/extensions/extension_sorting.cc?r1=130243&r2=130242&pathrev=130243

Merge 130040 - Fix Crash in ExtensionSorting::SyncIfNeeded

Replace CHECK with if statement, since it is possible to have old extensions that have ordinal values.


BUG= 120978 
TEST=Old profiles with extensions that have ordinal values don't crash on startup


Review URL: http://codereview.chromium.org/9956020

TBR=csharp@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9968066
------------------------------------------------------------------------

Comment 16 by k...@google.com, Aug 8 2012

Labels: -Merge-Approved
Remove merge approval label, this release has passed.
Project Member

Comment 17 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 18 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-UI -Feature-Sync -Mstone-19 -Feature-Extensions Cr-Services-Sync Cr-Platform-Extensions Cr-UI M-19
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 13 2013

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

Sign in to add a comment