Crash during forced devices poll while syncing

[AlexeyBarabash]

There is a crash from user, id is 61320200-802f-5e0b-0000-000000000000 .
Reported by @Brave-Matt

Stack is

[ 00 ] std::__Cr::unique_ptr<syncer::SyncInvalidation,std::__Cr::default_delete<syncer::SyncInvalidation> >::operator->() ( unique_ptr.h:274 )
[ 01 ] syncer::ModelTypeWorker::CollectPendingInvalidations(sync_pb::GetUpdateTriggers *) ( model_type_worker.cc:1288 )
[ 02 ] syncer::GetUpdatesProcessor::PrepareGetUpdates(base::EnumSet<syncer::ModelType,1,46> const &,sync_pb::ClientToServerMessage *) ( get_updates_processor.cc:202 )
[ 03 ] syncer::GetUpdatesProcessor::DownloadUpdates(base::EnumSet<syncer::ModelType,1,46> *,syncer::SyncCycle *) ( get_updates_processor.cc:189 )
[ 04 ] syncer::Syncer::DownloadAndApplyUpdates(base::EnumSet<syncer::ModelType,1,46> *,syncer::SyncCycle *,syncer::GetUpdatesDelegate const &) ( syncer.cc:180 )
[ 05 ] syncer::Syncer::BuildAndPostCommits::<lambda_0>::operator()(syncer::Syncer *,syncer::SyncCycle *,syncer::NudgeTracker *,syncer::BraveSyncerDevicePoll *) ( syncer.cc:248 )
[ 06 ] base::internal::FunctorTraits<`lambda at ..\..\..\src\components\sync\engine\syncer.cc:248:5',void>::Invoke(syncer::Syncer::BuildAndPostCommits::<lambda_0> &&,syncer::Syncer * &&,syncer::SyncCycle * &&,syncer::NudgeTracker * &&,syncer::BraveSyncerDevicePoll * &&) ( bind_internal.h:616 )
[ 07 ] base::internal::InvokeHelper<0,void,0,1,2,3>::MakeItSo(syncer::Syncer::BuildAndPostCommits::<lambda_0> &&,std::__Cr::tuple<base::internal::UnretainedWrapper<syncer::Syncer,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::SyncCycle,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::NudgeTracker,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::BraveSyncerDevicePoll,base::unretained_traits::MayNotDangle,0> > &&) ( bind_internal.h:868 )
[ 08 ] base::internal::Invoker<base::internal::BindState<`lambda at ..\..\..\src\components\sync\engine\syncer.cc:248:5',base::internal::UnretainedWrapper<syncer::Syncer,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::SyncCycle,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::NudgeTracker,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::BraveSyncerDevicePoll,base::unretained_traits::MayNotDangle,0> >,void ()>::RunImpl(syncer::Syncer::BuildAndPostCommits::<lambda_0> &&,std::__Cr::tuple<base::internal::UnretainedWrapper<syncer::Syncer,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::SyncCycle,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::NudgeTracker,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::BraveSyncerDevicePoll,base::unretained_traits::MayNotDangle,0> > &&,std::__Cr::integer_sequence<unsigned long long,0,1,2,3>) ( bind_internal.h:968 )
[ 09 ] base::internal::Invoker<base::internal::BindState<`lambda at ..\..\..\src\components\sync\engine\syncer.cc:248:5',base::internal::UnretainedWrapper<syncer::Syncer,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::SyncCycle,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::NudgeTracker,base::unretained_traits::MayNotDangle,0>,base::internal::UnretainedWrapper<syncer::BraveSyncerDevicePoll,base::unretained_traits::MayNotDangle,0> >,void ()>::RunOnce(base::internal::BindStateBase *) ( bind_internal.h:919 )
[ 10 ] base::OnceCallback<void ()>::Run() ( callback.h:154 )
[ 11 ] syncer::BraveSyncerDevicePoll::CheckIntervalAndPoll(base::OnceCallback<void ()>) ( brave_syncer_device_poll.cc:28 )
[ 12 ] syncer::Syncer::BuildAndPostCommits(base::EnumSet<syncer::ModelType,1,46> const &,syncer::NudgeTracker *,syncer::SyncCycle *) ( syncer.cc:249 )
[ 13 ] syncer::Syncer::NormalSyncShare(base::EnumSet<syncer::ModelType,1,46>,syncer::NudgeTracker *,syncer::SyncCycle *) ( syncer.cc:131 )
[ 14 ] syncer::SyncSchedulerImpl::DoNudgeSyncCycleJob(syncer::SyncSchedulerImpl::JobPriority) ( sync_scheduler_impl.cc:444 )
[ 15 ] syncer::SyncSchedulerImpl::TrySyncCycleJobImpl() ( sync_scheduler_impl.cc:683 )
[ 16 ] base::OnceCallback<void ()>::Run() ( callback.h:154 )
[ 17 ] base::TaskAnnotator::RunTaskImpl(base::PendingTask &) ( task_annotator.cc:201 )

There are ~4200 similar crashes contaning ModelTypeWorker::CollectPendingInvalidations or BraveSyncerDevicePoll calls at the stack.

The exact line of code where it happens is:

void ModelTypeWorker::CollectPendingInvalidations(
    sync_pb::GetUpdateTriggers* msg) {
  // Fill the list of payloads, if applicable.  The payloads must be ordered
  // oldest to newest, so we insert them in the same order as we've been storing
  // them internally.
  for (PendingInvalidation& invalidation : pending_invalidations_) {
    if (!invalidation.pending_invalidation->IsUnknownVersion()) {
//                                        ^
//                                        here

We don't use invalidation at all, I don't know how that line is even reached.
I wasn't able to reproduce it.

The code which is involved is from PR
brave/brave-core#17077
Issue it solved was #27931, after device creates the sync chain, it starts send the data and doesn't do pull, so it doesn't get updates about other devices at the chain. That PR added forced poll for for device type if commit takes too long.

Crash is worse than the delayed refresh of the synced device list, so the PR must be reverted, issue re-opened; it may be relaneded with some checks for pending_invalidation or other way to have forced device polling.

Also checked what if we poll devices on each commit - crash didn't happen.
And I tried to reproduce the crash with Use Google services for push messaging option from brave://settings/privacy - crash didn't happen.

[kjozwiak]

Changing the following to QA/Yes as we'll use the STR/Cases that were mentioned via brave/brave-core#21746 (comment). For example, similar to brave/brave-core#21746 (comment). @LaurenWags I think running through those cases once but including Win, macOS & Android as the devices should be good enough IMO.

[kjozwiak]

The above requires 1.62.156 or higher for 1.62.x verification 👍

[GeetaSarvadnya]

Verification PASSED on

Brave | 1.62.156 Chromium: 121.0.6167.139 (Official Build) (64-bit)
-- | --
Revision | 800674fc2c6162087525ed9b5bfc07230296b27d
OS | Windows 10 Version 22H2 (Build 19045.3930)

Bookmarks were retrieved from https://github.com/brave/brave-core/files/10678936/import_20k.zip via brave/brave-core#17077 (comment).

• Device #1 --> Win 10 x64 machine (imported bookmarks from the import1 folder)
• Device #2 --> macOS arm64 physical machine (imported bookmarks from the import2 folder)
• Device #3 --> Samsung Galaxy running Android 12 (added into the sync chain once Device #1 & Device #2 synced

STR/Cases:

• imported bookmarks from the import1 folder on Device #1
• imported bookmarks from the import2 folder on Device #2
• started a new sync chain via Device #1 and copied the Sync Chain Code
• added Device #2 into the sync chain by using the Sync Chain Code from Device #1

Once the above was completed, Device #2 appeared under Device #1 almost instantly. However, it took almost ~3min for Device #1 to appear under Device #2 which is expected due to reverting brave/brave-core#17077.

Device #2 (masOS arm64)

Example	Example	Example

Device #1 (Win 10 x64)

Example	Example	Example	Example

• once both Device #1 & Device #2 listed both devices via brave://settings/braveSync, added Device #3
  • scanned QR code from Device #1
• once scanned, Device #3 appeared on both Device #1 & Device #2 took almost ~5min

Device #3 (Samsung Galaxy running Android 12)

Example	Example	Example

[LaurenWags]

Verified with below version on various OSes/devices:

Brave | 1.62.156 Chromium: 121.0.6167.139 (Official Build) 

Bookmarks were retrieved from https://github.com/brave/brave-core/files/10678936/import_20k.zip via brave/brave-core#17077 (comment).

• Device #1 --> Win 10 x64 VM (imported bookmarks from the import1 folder)
• Device #2 --> macOS 13.x x64 machine (imported bookmarks from the import2 folder)
• Device #3 --> Pixel 3XL running Android 12 (added into the sync chain once Device #1 & Device #2 synced

STR/Cases:

• imported bookmarks from the import1 folder on Device #1
• imported bookmarks from the import2 folder on Device #2
• started a new sync chain via Device #1 and copied the Sync Chain Code
• added Device #2 into the sync chain by using the Sync Chain Code from Device #1

Once the above was completed, Device #1 appeared under Device #2 almost instantly. However, it took almost ~5min for Device #2 to appear under Device #1 which is expected due to reverting brave/brave-core#17077.

Device #2 (macOS 13.x x64)

Example	Example	Example

Device #1 (Win 10 x64)

Example	Example	Example

• once both Device #1 & Device #2 listed both devices via brave://settings/braveSync, added Device #3
  • scanned QR code from Device #1
• once scanned, Device #3 appeared on both Device #1 & Device #2 within ~1min
• bookmarks sync took a little longer, but did appear

Device #3 (Pixel 3 XL running Android 12)

Example	Example	Example	Device 1	Device 2

[kjozwiak]

Verification PASSED on Win 11 x64, Ubuntu 22.04 x64 & Pixel 6 running Android 14 using the following build(s):

Brave | 1.62.156 Chromium: 121.0.6167.139 (Official Build) (64-bit)
-- | --
Revision | 800674fc2c6162087525ed9b5bfc07230296b27d

Bookmarks were retrieved from https://github.com/brave/brave-core/files/10678936/import_20k.zip via brave/brave-core#17077 (comment).

• Device #1 --> Win 11 x64 machine (imported bookmarks from the import1 folder)
• Device #2 --> Ubuntu 22.04 x64 VM machine (imported bookmarks from the import2 folder)
• Device #3 --> Pixel 6 running Android 14 (added into the sync chain once Device #1 & Device #2 synced

STR/Cases:

• imported bookmarks from the import1 folder on Device #1
• imported bookmarks from the import2 folder on Device #2
• started a new sync chain via Device #1 and copied the Sync Chain Code
• added Device #2 into the sync chain by using the Sync Chain Code from Device #1

Once the above was completed, Device #1 appeared under Device #2 almost instantly. However, it took almost ~10min for Device #2 to appear under Device #1 which is expected due to reverting brave/brave-core#17077.

Device #2 (Ubuntu 22.04 x64)

Example	Example	Example

Device #1 (Win 11 x64)

Example	Example	Example

• once both Device #1 & Device #2 listed both devices via brave://settings/braveSync, added Device #3
  • scanned QR code from Device #1
• once scanned, Device #3 appeared on both Device #1 & Device #2 within ~3min

Device #3 (Pixel 6 running Android 14)

Example	Example	Example