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

Issue 852777 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

[local-sync] Turning on local sync on a profile using Google Sync causes a crash.

Project Member Reported by pastarmovj@chromium.org, Jun 14 2018

Issue description

When the profile was syncing against the Google Sync server and then the local sync backend was enabled. This leads to a crash for me in ProfileSyncService::OnActionableError when trying to call SigninManager::SignOut because the wrapper returns bogus pointer in the GetOriginal call. 

Related comment by msarda on the original bug: Is "SigninManagerWrapper" the wrapper that returns a bogus SigninManager? I think the SigninManagerWrapper is a class internal to sync, but it looks like a simple warpper, so the crash could come from it not being initialized.

Looking at https://cs.chromium.org/chromium/src/chrome/browser/sync/profile_sync_service_factory.cc?rcl=0a99285795ade2dd281995dee72fdb9e80d3fd6d&l=231 , it looks like the wrapper is not initialized at all when local_sync_backend_enabled is false. Would this explain the crash?
 

Comment 1 by treib@chromium.org, Jun 14 2018

Components: Services>Sync
Labels: OS-Windows
Status: Assigned (was: Available)
The SigninManagerWrapper these days just bundles a SigninManager and an IdentityManager. (It used to do more things for supervised users, but that's all gone.)

In local Sync mode, the wrapper will be null, and so the actual SigninManager* you get from it will be undefined. But the SignOut() call is wrapped in an "if (!IsLocalSyncEnabled())", so I don't quite see how the crash would occur? AFAIK the IsLocalSyncEnabled state can't change at runtime, right?

This bug might not be relevant anymore too. I just extracted the part why the other one was still open. Maybe this crash is not possible in the current version of Chrome.

Comment 3 by mastiz@chromium.org, Jun 27 2018

I think I could repro this issue, by enabling local sync in a previously-existing profile (is that a supported scenario?):
54210:54345:0627/071353.191538:WARNING:loopback_server.cc(638)] Loopback sync persistent state file does not exist.
[54210:54210:0627/071353.275587:FATAL:processor_entity_tracker.cc(238)] Check failed: commit_only || data.response_version > metadata_.server_version(). 5 vs 20
#0 0x7fce7ff5e7cd base::debug::StackTrace::StackTrace()
#1 0x7fce7fca436c base::debug::StackTrace::StackTrace()
#2 0x7fce7fd133aa logging::LogMessage::~LogMessage()
#3 0x5629a2bccfc6 syncer::ProcessorEntityTracker::ReceiveCommitResponse()
#4 0x5629a2b962ff syncer::ClientTagBasedModelTypeProcessor::OnCommitCompleted()
#5 0x5629a087e9ef _ZN4base8internal13FunctorTraitsIMN10extensions11ImageLoaderEFvRKNS_17RepeatingCallbackIFvN3gfx11ImageFamilyEEEERKNSt3__16vectorINS3_10LoadResultENSB_9allocatorISD_EEEEEvE6InvokeISK_RKNS_7WeakPtrIS3_EEJSA_SI_EEEvT_OT0_DpOT1_
#6 0x5629a087e945 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN10extensions11ImageLoaderEFvRKNS_17RepeatingCallbackIFvN3gfx11ImageFamilyEEEERKNSt3__16vectorINS5_10LoadResultENSD_9allocatorISF_EEEEERKNS_7WeakPtrIS5_EEJSC_SK_EEEvOT_OT0_DpOT1_
#7 0x5629a2bac60d _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer18ModelTypeProcessorEFvRKN7sync_pb14ModelTypeStateERKNSt3__16vectorINS3_18CommitResponseDataENS9_9allocatorISB_EEEEEJNS_7WeakPtrIS4_EES6_SE_EEEFvvEE7RunImplIRKSI_RKNS9_5tupleIJSK_S6_SE_EEEJLm0ELm1ELm2EEEEvOT_OT0_NS9_16integer_sequenceImJXspT1_EEEE
#8 0x5629a2bac4dc _ZN4base8internal7InvokerINS0_9BindStateIMN6syncer18ModelTypeProcessorEFvRKN7sync_pb14ModelTypeStateERKNSt3__16vectorINS3_18CommitResponseDataENS9_9allocatorISB_EEEEEJNS_7WeakPtrIS4_EES6_SE_EEEFvvEE3RunEPNS0_13BindStateBaseE
#9 0x7fce7fc5354e _ZNO4base12OnceCallbackIFvvEE3RunEv

While trying for the second time, I couldn't repro, and instead sync detected a mismatching birthday so it likely stripped out all sync metadata (preventing the DCHECK).

Sign in to add a comment