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

Issue 673508 link

Starred by 1 user
Status: Fixed
Owner:
s...@chromium.org
Closed: Dec 2016
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 650725



Sign in to add a comment

[USS] ModelTypeStore LevelDB is created before backend deletes sync folder

Project Member Reported by s...@chromium.org, Dec 12 2016 
It seems to only affect Windows, but right now we pass a callback to DeviceInfoSyncBridge to create the LevelDB [1] before we get the callback from the engine telling us that it is initialized. This is a big problem, because part of what the engine does is delete the entire Sync Data/ folder, which contains the LevelDB [2]. This creates a race condition that Windows integ tests are running into [3].

[1] https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_service.cc?q=DeviceInfoSyncBridge&sq=package:chromium&l=313&dr=C

[2] https://cs.chromium.org/chromium/src/components/sync/driver/glue/sync_backend_host_core.cc?sq=package:chromium&dr=CSs&l=380

[3] https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/331517/steps/compile%20%28with%20patch%29/logs/stdio
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 20 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf

commit 4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf
Author: skym <skym@chromium.org>
Date: Tue Dec 20 17:29:37 2016

[Sync] Stop deleting LevelDB files when deleting Directory

The LevelDB folder backing ModelTypeStore is inside Sync Data folder.
While this change is being driven by race conditions from sync trying
to delete this folder while it is being used, this setup is fundamental
flawed. The ModelTypeStores will hold model type specific information
that needs to be persistent across sign outs and disabling sync.

The approach this CL takes to fix this problem is the modify the
deletion logic to only affect the Directory (sqllite3) files. This
means that the Sync Data folder will still exist even when sync is off.

BUG= 673508 , 673887 

Review-Url: https://codereview.chromium.org/2568743004
Cr-Commit-Position: refs/heads/master@{#439835}

[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/chrome/browser/sync/test/integration/single_client_directory_sync_test.cc
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/browser_sync/profile_sync_components_factory_impl.cc
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/browser_sync/profile_sync_components_factory_impl.h
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/browser_sync/profile_sync_service.h
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/driver/glue/sync_backend_host_core.cc
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/driver/glue/sync_backend_host_core.h
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/driver/glue/sync_backend_host_impl.cc
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/driver/glue/sync_backend_host_impl.h
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/driver/sync_service_base.cc
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/driver/sync_service_base.h
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/syncable/directory.cc
[modify] https://crrev.com/4234d3c65dc2b0ad0cf19fbf5e9e1e528bc3ddaf/components/sync/syncable/directory.h

Comment 2 by s...@chromium.org, Dec 20 2016

Blocking: -672596

Comment 3 by s...@chromium.org, Dec 20 2016

Blocking: 650725

Comment 4 by s...@chromium.org, Dec 21 2016

Status: Fixed (was: Started)
►

Sign in to add a comment