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

Issue metadata

Status: Fixed
Owner:
Closed: Dec 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 843511
issue 917380
issue 917572



Sign in to add a comment
link

Issue 852002: sync_integration_tests doesn't pass under tsan

Reported by thakis@chromium.org, Jun 12 2018 Project Member

Issue description

I gave running sync_integration_tests under tsan a try: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_tsan_rel_ng/36372

failures:
TwoClientDictionarySyncTest.Limit
TwoClientAppsSyncTest.UpdateCWSOrdinals_E2ETest
TwoClientBookmarksSyncTest.SC_MovingBMsFromBMFoldToBMBar
TwoClientExtensionsSyncTest.StartWithDifferentExtensions_E2ETest
TwoClientPasswordsSyncTest.Merge_E2ETest
TwoClientPreferencesSyncTest.BooleanPref_E2ETest
TwoClientBookmarksSyncTest.SC_MovingBMsFromBMBarToBMFolder
TwoClientPreferencesSyncTest.SingleClientEnabledEncryptionBothChanged_E2ETest
TwoClientPasswordsSyncTest.DeleteAll
TwoClientBookmarksSyncTest.SC_MovingBMsFromParentBMFoldToChildBMFold
TwoClientExtensionsSyncTest.InstallDifferentExtensions_E2ETest
TwoClientBookmarksSyncTest.SC_SinkBMs10LevelDown
TwoClientExtensionSettingsAndAppSettingsSyncTest.ExtensionsStartWithSameSettings
TwoClientPreferencesSyncTestWithSelfNotifications.LateRegisteredPrefsShouldSync_E2ETest
TwoClientExtensionsSyncTest.Uninstall_E2ETest
TwoClientTypedUrlsSyncTest.AddExpired
TwoClientBookmarksSyncTest.MC_SimpleMergeOfDifferentBMModels
TwoClientPasswordsSyncTest.Delete
TwoClientBookmarksSyncTest.SC_SinkEmptyBMFold5LevelsDown
TwoClientDictionarySyncTest.Sanity_E2ETest
TwoClientAutofillSyncTest.DeleteProfile
TwoClientDictionarySyncTest.AddDifferentToEach_E2ETest
TwoClientAutofillSyncTest.ConflictingFields
TwoClientThemesSyncTest.CustomThenSyncNative_E2ETest
TwoClientPreferencesSyncTest.UnsyncableBooleanPref_E2ETest
TwoClientAutofillSyncTest.MaxLength
TwoClientDictionarySyncTest.RemoveOnAAddOnB_E2ETest
TwoClientPasswordsSyncTest.Update
MigrationTwoClientTest.MigratePrefsAndBookmarksThenModifyBookmark
TwoClientPreferencesSyncTest.BothClientsEnabledEncryptionAndChangedMultipleTimes_E2ETest
TwoClientPasswordsSyncTest.Race_E2ETest
TwoClientAutofillSyncTest.AddMultipleProfiles
TwoClientAppsSyncTest.Merge_E2ETest
TwoClientSearchEnginesSyncTest.MergeMultiple
TwoClientThemesSyncTest.CustomThenSyncDefault_E2ETest
TwoClientAppsSyncTest.InstallDifferentApps_E2ETest
TwoClientAppsSyncTest.Add_E2ETest
TwoClientExtensionSettingsAndAppSettingsSyncTest.AppsStartWithDifferentSettings
TwoClientBookmarksSyncTest.SC_DelLastBMUnderBMFoldNonEmptyFoldAfterwards
TwoClientBookmarksSyncTest.OneClientAddsFolderAndBookmark
TwoClientDictionarySyncTest.SimultaneousAdd_E2ETest
TwoClientBookmarksSyncTest.SC_HoistBMs10LevelUp
TwoClientExtensionSettingsAndAppSettingsSyncTest.AppsStartWithSameSettings
TwoClientExtensionsSyncTest.Add_E2ETest
TwoClientAppsSyncTest.UpdateEnableDisableApp_E2ETest
TwoClientAppsSyncTest.StartWithDifferentApps
TwoClientAutofillSyncTest.AddProfile
TwoClientExtensionSettingsAndAppSettingsSyncTest.ExtensionsStartWithDifferentSettings
TwoClientBookmarksSyncTest.DisableSync
TwoClientSearchEnginesSyncTest.Add_E2ETest
TwoClientBookmarksSyncTest.SC_RenameBMFolderThatHasParentAndChildren
TwoClientBookmarksSyncTest.SC_DelMiddleBMUnderBMFoldNonEmptyFoldAfterwards
TwoClientDictionarySyncTest.SimultaneousRemove_E2ETest
TwoClientBookmarksSyncTest.MC_FolderNameChangeConflict
TwoClientAppsSyncTest.UpdateLaunchType_E2ETest
TwoClientExtensionsSyncTest.UpdateEnableDisableExtension_E2ETest
TwoClientBookmarksSyncTest.SC_RenameBMFolderWithLongHierarchy
TwoClientUssSyncTest.Encryption
TwoClientPasswordsSyncTest.Add_E2ETest
TwoClientBookmarksSyncTest.SC_RenameBMNameAndURL
TwoClientSearchEnginesSyncTest.ConflictKeyword
TwoClientExtensionsSyncTest.UpdateIncognitoEnableDisable_E2ETest
TwoClientBookmarksSyncTest.SC_AddSeveralBMsAndFolders
TwoClientBookmarksSyncTest.MC_BiDirectionalPushAddingBM
TwoClientAppsSyncTest.UpdateIncognitoEnableDisable_E2ETest
TwoClientBookmarksSyncTest.SC_ReverseTheOrderOfTenBMFolders
TwoClientAutofillSyncTest.ExceedsMaxLength



This stack here shows up in at least two stacks:

yncer::ModelType, (syncer::ModelType)2, (syncer::ModelType)41>, bool) chrome/browser/sync/test/integration/profile_sync_service_harness.cc:187:14 (sync_integration_tests+0x6664477)
    #16 ProfileSyncServiceHarness::SetupSync() chrome/browser/sync/test/integration/profile_sync_service_harness.cc:128:17 (sync_integration_tests+0x6663dfe)
    #17 SyncTest::SetupSync() chrome/browser/sync/test/integration/sync_test.cc:695:34 (sync_integration_tests+0x667fea6)
    #18 TwoClientBookmarksSyncTest_SC_MovingBMsFromParentBMFoldToChildBMFold_Test::RunTestOnMainThread() chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc:919:3 (sync_integration_tests+0x289e383)
    #19 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() content/public/test/browser_test_base.cc:385:5 (sync_integration_tests+0xf1f7a94)
    #20 Invoke<void (content::BrowserTestBase::*)(), content::BrowserTestBase *> base/bind_internal.h:507:12 (sync_integration_tests+0xf1f88b8)
    #21 MakeItSo<void (content::BrowserTestBase::*const &)(), content::BrowserTestBase *> base/bind_internal.h:607 (sync_integration_tests+0xf1f88b8)
    #22 RunImpl<void (content::BrowserTestBase::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::BrowserTestBase> > &, 0> base/bind_internal.h:681 (sync_integration_tests+0xf1f88b8)
    #23 base::internal::Invoker<base::internal::BindState<void (content::BrowserTestBase::*)(), base::internal::UnretainedWrapper<content::BrowserTestBase> >, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:663 (sync_integration_tests+0xf1f88b8)
    #24 Run base/callback.h:125:12 (sync_integration_tests+0x6e0868f)
    #25 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:2137 (sync_integration_tests+0x6e0868f)
    #26 ChromeBrowserMainParts::PreMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:1521:18 (sync_integration_tests+0x6e06eab)
    #27 content::BrowserMainLoop::PreMainMessageLoopRun() content/browser/browser_main_loop.cc:966:13 (sync_integration_tests+0x3ec5e00)
    #28 Invoke<int (content::BrowserMainLoop::*)(), content::BrowserMainLoop *> base/bind_internal.h:507:12 (sync_integration_tests+0x3ec9898)
    #29 MakeItSo<int (content::BrowserMainLoop::*const &)(), content::BrowserMainLoop *> base/bind_internal.h:607 (sync_integration_tests+0x3ec9898)
    #30 RunImpl<int (content::BrowserMainLoop::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::BrowserMainLoop> > &, 0> base/bind_internal.h:681 (sync_integration_tests+0x3ec9898)
    #31 base::internal::Invoker<base::internal::BindState<int (content::BrowserMainLoop::*)(), base::internal::UnretainedWrapper<content::BrowserMainLoop> >, int ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:663 (sync_integration_tests+0x3ec9898)
    #32 Run base/callback.h:125:12 (sync_integration_tests+0x4685bfa)
    #33 content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:44 (sync_integration_tests+0x4685bfa)
    #34 content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:877:25 (sync_integration_tests+0x3ec3ea3)
    #35 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) content/browser/browser_main_runner_impl.cc:148:15 (sync_integration_tests+0x3ec9f01)
    #36 content::BrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) content/browser/browser_main.cc:47:20 (sync_integration_tests+0x3ec0dc6)
    #37 content::RunBrowserProcessMain(content::MainFunctionParams const&, content::ContentMainDelegate*, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) content/app/content_main_runner_impl.cc:620:10 (sync_integration_tests+0x6607383)
    #38 content::ContentMainRunnerImpl::Run() content/app/content_main_runner_impl.cc:964:12 (sync_integration_tests+0x660875e)
    #39 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() content/app/content_service_manager_main_delegate.cc:53:32 (sync_integration_tests+0x6604c4f)
    #40 service_manager::Main(service_manager::MainParams const&) services/service_manager/embedder/main.cc:459:29 (sync_integration_tests+0x95d6ade)
    #41 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:10 (sync_integration_tests+0x6605f4e)
    #42 content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:323:3 (sync_integration_tests+0xf1f74f0)
    #43 InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:244:20 (sync_integration_tests+0x669023a)
    #44 SyncTest::SetUp() chrome/browser/sync/test/integration/sync_test.cc:258:25 (sync_integration_tests+0x667b479)
    #45 testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc (sync_integration_tests+0x2a812bb)
    #46 testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2667:11 (sync_integration_tests+0x2a8235c)
    #47 testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2785:28 (sync_integration_tests+0x2a82be6)
    #48 testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5047:43 (sync_integration_tests+0x2a93186)
    #49 testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc (sync_integration_tests+0x2a92a6a)
    #50 RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2329:46 (sync_integration_tests+0x688dfe5)
    #51 base::TestSuite::Run() base/test/test_suite.cc:275 (sync_integration_tests+0x688dfe5)
    #52 ChromeTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/chrome_test_launcher.cc:65:38 (sync_integration_tests+0x668efa6)
    #53 ChromeTestLauncherDelegate::RunTestSuite(int, char**) chrome/test/base/chrome_test_launcher.cc:74:19 (sync_integration_tests+0x668f0af)
    #54 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:625:31 (sync_integration_tests+0xf1fe599)
    #55 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:170:10 (sync_integration_tests+0x668f634)
    #56 main chrome/test/base/browser_tests_main.cc:36:10 (sync_integration_tests+0x664061c)
  Thread T34 'Chrome_SyncThread' (tid=17949, running) created by main thread at:
    #0 pthread_create /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:965:3 (sync_integration_tests+0x27aefe5)
    #1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:115:13 (sync_integration_tests+0x69ed1c6)
    #2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:200:10 (sync_integration_tests+0x69ed085)
    #3 base::Thread::StartWithOptions(base::Thread::Options const&) base/threading/thread.cc:112:15 (sync_integration_tests+0x6986a25)
    #4 browser_sync::ProfileSyncService::InitializeEngine() components/browser_sync/profile_sync_service.cc:579:34 (sync_integration_tests+0xa899cc8)
    #5 browser_sync::ProfileSyncService::StartUpSlowEngineComponents() components/browser_sync/profile_sync_service.cc:559:3 (sync_integration_tests+0xa899b19)
    #6 Invoke<void (browser_sync::ProfileSyncService::*)(), const base::WeakPtr<browser_sync::ProfileSyncService> &> base/bind_internal.h:507:12 (sync_integration_tests+0xa8a8352)
    #7 MakeItSo<void (browser_sync::ProfileSyncService::*const &)(), const base::WeakPtr<browser_sync::ProfileSyncService> &> base/bind_internal.h:627 (sync_integration_tests+0xa8a8352)
    #8 RunImpl<void (browser_sync::ProfileSyncService::*const &)(), const std::__1::tuple<base::WeakPtr<browser_sync::ProfileSyncService> > &, 0> base/bind_internal.h:681 (sync_integration_tests+0xa8a8352)
    #9 base::internal::Invoker<base::internal::BindState<void (browser_sync::ProfileSyncService::*)(), base::WeakPtr<browser_sync::ProfileSyncService> >, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:663 (sync_integration_tests+0xa8a8352)
    #10 Run base/callback.h:125:12 (sync_integration_tests+0xa8ad602)
    #11 syncer::StartupController::StartUp(syncer::StartupController::StartUpDeferredOption) components/sync/driver/startup_controller.cc:105 (sync_integration_tests+0xa8ad602)
    #12 syncer::StartupController::TryStart() components/sync/driver/startup_controller.cc (sync_integration_tests+0xa8ad48c)
    #13 syncer::StartupController::TryStartImmediately() components/sync/driver/startup_controller.cc:136:3 (sync_integration_tests+0xa8ada51)
    #14 browser_sync::ProfileSyncService::RequestStart() components/browser_sync/profile_sync_service.cc:1992:24 (sync_integration_tests+0xa8a6575)
    #15 ProfileSyncServiceHarness::SetupSync(syncer::EnumSet<syncer::ModelType, (syncer::ModelType)2, (syncer::ModelType)41>, bool) chrome/browser/sync/test/integration/profile_sync_service_harness.cc:187:14 (sync_integration_tests+0x6664477)
    #16 ProfileSyncServiceHarness::SetupSync() chrome/browser/sync/test/integration/profile_sync_service_harness.cc:128:17 (sync_integration_tests+0x6663dfe)
    #17 SyncTest::SetupSync() chrome/browser/sync/test/integration/sync_test.cc:695:34 (sync_integration_tests+0x667fea6)
    #18 TwoClientBookmarksSyncTest_SC_MovingBMsFromParentBMFoldToChildBMFold_Test::RunTestOnMainThread() chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc:919:3 (sync_integration_tests+0x289e383)
    #19 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() content/public/test/browser_test_base.cc:385:5 (sync_integration_tests+0xf1f7a94)
    #20 Invoke<void (content::BrowserTestBase::*)(), content::BrowserTestBase *> base/bind_internal.h:507:12 (sync_integration_tests+0xf1f88b8)
    #21 MakeItSo<void (content::BrowserTestBase::*const &)(), content::BrowserTestBase *> base/bind_internal.h:607 (sync_integration_tests+0xf1f88b8)
    #22 RunImpl<void (content::BrowserTestBase::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::BrowserTestBase> > &, 0> base/bind_internal.h:681 (sync_integration_tests+0xf1f88b8)
    #23 base::internal::Invoker<base::internal::BindState<void (content::BrowserTestBase::*)(), base::internal::UnretainedWrapper<content::BrowserTestBase> >, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:663 (sync_integration_tests+0xf1f88b8)
    #24 Run base/callback.h:125:12 (sync_integration_tests+0x6e0868f)
    #25 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:2137 (sync_integration_tests+0x6e0868f)
    #26 ChromeBrowserMainParts::PreMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:1521:18 (sync_integration_tests+0x6e06eab)
    #27 content::BrowserMainLoop::PreMainMessageLoopRun() content/browser/browser_main_loop.cc:966:13 (sync_integration_tests+0x3ec5e00)
    #28 Invoke<int (content::BrowserMainLoop::*)(), content::BrowserMainLoop *> base/bind_internal.h:507:12 (sync_integration_tests+0x3ec9898)
    #29 MakeItSo<int (content::BrowserMainLoop::*const &)(), content::BrowserMainLoop *> base/bind_internal.h:607 (sync_integration_tests+0x3ec9898)
    #30 RunImpl<int (content::BrowserMainLoop::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::BrowserMainLoop> > &, 0> base/bind_internal.h:681 (sync_integration_tests+0x3ec9898)
    #31 base::internal::Invoker<base::internal::BindState<int (content::BrowserMainLoop::*)(), base::internal::UnretainedWrapper<content::BrowserMainLoop> >, int ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:663 (sync_integration_tests+0x3ec9898)
    #32 Run base/callback.h:125:12 (sync_integration_tests+0x4685bfa)
    #33 content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:44 (sync_integration_tests+0x4685bfa)
    #34 content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:877:25 (sync_integration_tests+0x3ec3ea3)
    #35 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) content/browser/browser_main_runner_impl.cc:148:15 (sync_integration_tests+0x3ec9f01)
    #36 content::BrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) content/browser/browser_main.cc:47:20 (sync_integration_tests+0x3ec0dc6)
    #37 content::RunBrowserProcessMain(content::MainFunctionParams const&, content::ContentMainDelegate*, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) content/app/content_main_runner_impl.cc:620:10 (sync_integration_tests+0x6607383)
    #38 content::ContentMainRunnerImpl::Run() content/app/content_main_runner_impl.cc:964:12 (sync_integration_tests+0x660875e)
    #39 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() content/app/content_service_manager_main_delegate.cc:53:32 (sync_integration_tests+0x6604c4f)
    #40 service_manager::Main(service_manager::MainParams const&) services/service_manager/embedder/main.cc:459:29 (sync_integration_tests+0x95d6ade)
    #41 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:10 (sync_integration_tests+0x6605f4e)
    #42 content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:323:3 (sync_integration_tests+0xf1f74f0)
    #43 InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:244:20 (sync_integration_tests+0x669023a)
    #44 SyncTest::SetUp() chrome/browser/sync/test/integration/sync_test.cc:258:25 (sync_integration_tests+0x667b479)
    #45 testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc (sync_integration_tests+0x2a812bb)
    #46 testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2667:11 (sync_integration_tests+0x2a8235c)
    #47 testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2785:28 (sync_integration_tests+0x2a82be6)
    #48 testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5047:43 (sync_integration_tests+0x2a93186)
    #49 testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc (sync_integration_tests+0x2a92a6a)
    #50 RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2329:46 (sync_integration_tests+0x688dfe5)
    #51 base::TestSuite::Run() base/test/test_suite.cc:275 (sync_integration_tests+0x688dfe5)
    #52 ChromeTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/chrome_test_launcher.cc:65:38 (sync_integration_tests+0x668efa6)
    #53 ChromeTestLauncherDelegate::RunTestSuite(int, char**) chrome/test/base/chrome_test_launcher.cc:74:19 (sync_integration_tests+0x668f0af)
    #54 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:625:31 (sync_integration_tests+0xf1fe599)
    #55 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:170:10 (sync_integration_tests+0x668f634)
    #56 main chrome/test/base/browser_tests_main.cc:36:10 (sync_integration_tests+0x664061c)
SUMMARY: ThreadSanitizer: data race gen/components/sync/protocol/sync.pb.cc:2684:17 in sync_pb::EntitySpecifics::ByteSizeLong() const
 

Comment 1 by mastiz@chromium.org, Jun 12 2018

After taking a quick look, some of the stack traces I see are generated proto code that is wrapped within GOOGLE_SAFE_CONCURRENT_WRITES_BEGIN()/END().

Does tsan honor that?

Comment 2 by thakis@chromium.org, Jun 12 2018

Cc: glider@chromium.org
glider, do you know what tsan thinks about GOOGLE_SAFE_CONCURRENT_WRITES_BEGIN?

We do use proto code in other binaries we run under tsan, so there's probably some way to make this work?

Comment 3 by thakis@chromium.org, Jun 12 2018

We could maybe do -DGOOGLE_SAFE_CONCURRENT_WRITES_BEGIN=AnnotateIgnoreWritesBegin(__FILE__, __LINE__) but it's weird it's not needed anywhere else.

Comment 4 by glider@chromium.org, Jun 12 2018

I'll need to take a closer look when in the office, but overall we don't
want any annotations to work.

Comment 5 by glider@chromium.org, Jun 13 2018

Cc: dvyukov@chromium.org xyzzyz@chromium.org pkasting@chromium.org
The race report in #0 is garbled, but it's pretty obvious that every SetCachedSize() implementation in Chromium has a write/write data race in it.
Does anyone know which code generates those functions? I couldn't easily find it on cs.chromium.org by searching for GOOGLE_SAFE_CONCURRENT_WRITES_BEGIN etc.

Comment 7 by glider@chromium.org, Jun 13 2018

I'm gonna discuss this with the protobuf team, we'd better remove all these annotations.
For the time being we can suppress races in SetCachedSize() in Chromium. I don't think it's a good idea to add support for GOOGLE_SAFE_CONCURRENT_WRITES_BEGIN, as other occurrences of it may show up without us noticing.

Comment 8 by mastiz@chromium.org, Jun 13 2018

Labels: Sync-Triaged
Owner: glider@chromium.org
Status: Assigned (was: Untriaged)
Assigning explicitly for glider@ to follow up, thanks.

Comment 9 by thakis@chromium.org, Jun 21 2018

Cc: thomasanderson@chromium.org
This got removed from public protobuf in https://github.com/google/protobuf/commit/0400cca3236de1ca303af38bf81eab332d042b7c a few months ago.

Should we cherry-pick that one (it's pretty large), or should we try and sync up protobuf again?

Comment 10 by thomasanderson@chromium.org, Jun 21 2018

I'd recommend cherry-picking the internal change (not the large public one) since we have access to it.  Updating protobuf is a surprisingly tricky process.

Comment 11 by pkasting@chromium.org, Jun 21 2018

Please try to update protobufs over cherry-picking changes, as the longer we do the latter instead of the former the worse things get.

Comment 12 by pkasting@chromium.org, Jun 21 2018

There's actually someone trying to do that right now on https://chromium-review.googlesource.com/c/chromium/src/+/1081411 but having some trouble -- maybe reach out and work together to push it through.

Comment 13 by glider@chromium.org, Jun 26 2018

Owner: ----
Status: Available (was: Assigned)
As Nico said, this isn't in protobuf anymore.
Marking as "Available"

Comment 14 by thakis@chromium.org, Jun 26 2018

Looks like there are still references in public protobuf to this after all.

(internal issue b/110398348)

Comment 15 by thakis@chromium.org, Aug 31

Cc: medinaan...@chromium.org
Finally merged to public in https://github.com/protocolbuffers/protobuf/commit/6bbe197e9c1b6fc38cbdc45e3bf83fa7ced792a3 (changes to cpp_enum_field.cc cpp_primitive_field.cc descriptor.pb.cc dynamic_message.cc). This landed on Aug 8.

https://chromium.googlesource.com/chromium/src/third_party/+/51d79a0106aeac2ef77910ef4cfbef546b0647e4 in  issue 874265  updated us to protbuf cf242503, which is from May 30. So we're still a bit over 2 months too far back and need either another roll or a cherry pick of that commit (which is pretty massive).

medinaandres, was there a reason why you didn't roll further than cf242503? Or did you just not need more?

Comment 16 by medinaandres@google.com, Aug 31

So I took the ToT when I started the effort (around May 2018). Didn't move forward in the middle of the uprev as there were enough hassles to solve with a stable copy.

Comment 17 by thakis@chromium.org, Aug 31

Ah ok. I figured you filed the bug before you started, sounds like that assumption was wrong :-)

Comment 18 by thakis@chromium.org, Dec 14

Owner: thakis@chromium.org
Status: Started (was: Available)
For some reason, only two tests of this suite still fail. I'm disabling these two (follow-up work for reenabling them is tracked in  issue 915219 ) and I'm adding sync_integration_tests to the tsan bot in https://chromium-review.googlesource.com/c/chromium/src/+/1377327/

Comment 19 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8fbc2838d0dc12de17f79bb2a760ad859fc41a76

commit 8fbc2838d0dc12de17f79bb2a760ad859fc41a76
Author: Nico Weber <thakis@chromium.org>
Date: Fri Dec 14 16:00:57 2018

Disable the three sync_integration_tests that fail under tsan and run suite on tsan bot.

Looks like all the protobuf issues went away, maybe due to #612796.

Only some tests still fail:
- TwoClientCustomPassphraseSyncTest.ClientsCanSyncDataWhenScryptEncryptionEnabledInOne
- EnableDisableSingleClientTest.DoesNotRedownloadAfterKeepDataWithStandaloneTransport
- SingleClientDirectorySyncTest.StopThenDisableDeletesDirectory

Disable them under tsan and run the suite on the tsan bot.

Bug:  852002 , 915219 ,843511, 913704 
Change-Id: I862a93a4669c2dbd57ded99c9704528201f687b9
Reviewed-on: https://chromium-review.googlesource.com/c/1377327
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616693}
[modify] https://crrev.com/8fbc2838d0dc12de17f79bb2a760ad859fc41a76/chrome/browser/sync/test/integration/enable_disable_test.cc
[modify] https://crrev.com/8fbc2838d0dc12de17f79bb2a760ad859fc41a76/chrome/browser/sync/test/integration/single_client_directory_sync_test.cc
[modify] https://crrev.com/8fbc2838d0dc12de17f79bb2a760ad859fc41a76/chrome/browser/sync/test/integration/two_client_custom_passphrase_sync_test.cc
[modify] https://crrev.com/8fbc2838d0dc12de17f79bb2a760ad859fc41a76/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/8fbc2838d0dc12de17f79bb2a760ad859fc41a76/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/8fbc2838d0dc12de17f79bb2a760ad859fc41a76/testing/buildbot/test_suite_exceptions.pyl

Comment 21 by thakis@chromium.org, Dec 28

Blocking: 917572

Comment 22 by thakis@chromium.org, Dec 28

Blocking: 917380

Sign in to add a comment