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

Issue 753030 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 368525



Sign in to add a comment

Data race in PersistentHistogramAllocator::GetCreateHistogramResultHistogram

Project Member Reported by gab@chromium.org, Aug 7 2017

Issue description

I think it has to do with :

    // It's possible for multiple threads to make it here in parallel but
    // they'll always return the same result as there is a mutex in the Get.
    // The purpose of the "initialized" variable is just to ensure that
    // the same thread doesn't recurse which is also why it doesn't have
    // to be atomic.
    static bool initialized = false;
    if (!initialized) {
      initialized = true;
      (...)

despite the comment, this doesn't look thread-safe to me. Again, there's no such thing as a benign data race, if this only matters as a thread-local value, you can use a ThreadLocalStorage::Slot but a static will propagate across threads and not using atomics results in undefined behavior.


WARNING: ThreadSanitizer: data race (pid=25175)
  Read of size 1 at 0x000010694ee0 by main thread:
    #0 base::PersistentHistogramAllocator::GetCreateHistogramResultHistogram() base/metrics/persistent_histogram_allocator.cc (browser_tests+0x42f9421)
    #1 RecordCreateHistogramResult base/metrics/persistent_histogram_allocator.cc:741:37 (browser_tests+0x42f802a)
    #2 base::PersistentHistogramAllocator::CreateHistogram(base::PersistentHistogramAllocator::PersistentHistogramData*) base/metrics/persistent_histogram_allocator.cc:697 (browser_tests+0x42f802a)
    #3 base::PersistentHistogramAllocator::AllocateHistogram(base::HistogramType, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, int, base::BucketRanges const*, int, unsigned int*) base/metrics/persistent_histogram_allocator.cc:412:48 (browser_tests+0x42f888e)
    #4 base::Histogram::Factory::Build() base/metrics/histogram.cc:186:40 (browser_tests+0x42eddf4)
    #5 base::Histogram::FactoryGet(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, int, unsigned int, int) base/metrics/histogram.cc:253:63 (browser_tests+0x42ee282)
    #6 base::Histogram::FactoryGet(char const*, int, int, unsigned int, int) base/metrics/histogram.cc:271:10 (browser_tests+0x42ee99c)
    #7 blink::scheduler::RendererSchedulerImpl::RecordTaskMetrics(blink::scheduler::MainThreadTaskQueue::QueueType, base::TimeTicks, base::TimeTicks) third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:1997:3 (browser_tests+0x40c8751)
    #8 blink::scheduler::RendererSchedulerImpl::OnTaskCompleted(blink::scheduler::MainThreadTaskQueue*, blink::scheduler::TaskQueue::Task const&, base::TimeTicks, base::TimeTicks) third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:1959:3 (browser_tests+0x40c83cc)
    #9 blink::scheduler::MainThreadTaskQueue::OnTaskCompleted(blink::scheduler::TaskQueue::Task const&, base::TimeTicks, base::TimeTicks) third_party/WebKit/Source/platform/scheduler/renderer/main_thread_task_queue.cc:100:24 (browser_tests+0x40b75bd)
    #10 Invoke<blink::scheduler::MainThreadTaskQueue *, const blink::scheduler::TaskQueue::Task &, base::TimeTicks, base::TimeTicks> base/bind_internal.h:196:12 (browser_tests+0x40b7742)
    #11 MakeItSo<void (blink::scheduler::MainThreadTaskQueue::*const &)(const blink::scheduler::TaskQueue::Task &, base::TimeTicks, base::TimeTicks), blink::scheduler::MainThreadTaskQueue *, const blink::scheduler::TaskQueue::Task &, base::TimeTicks, base::TimeTicks> base/bind_internal.h:265 (browser_tests+0x40b7742)
    #12 RunImpl<void (blink::scheduler::MainThreadTaskQueue::*const &)(const blink::scheduler::TaskQueue::Task &, base::TimeTicks, base::TimeTicks), const std::__1::tuple<base::internal::UnretainedWrapper<blink::scheduler::MainThreadTaskQueue> > &, 0> base/bind_internal.h:340 (browser_tests+0x40b7742)
    #13 base::internal::Invoker<base::internal::BindState<void (blink::scheduler::MainThreadTaskQueue::*)(blink::scheduler::TaskQueue::Task const&, base::TimeTicks, base::TimeTicks), base::internal::UnretainedWrapper<blink::scheduler::MainThreadTaskQueue> >, void (blink::scheduler::TaskQueue::Task const&, base::TimeTicks, base::TimeTicks)>::Run(base::internal::BindStateBase*, blink::scheduler::TaskQueue::Task const&, base::TimeTicks&&, base::TimeTicks&&) base/bind_internal.h:319 (browser_tests+0x40b7742)
    #14 Run base/callback.h:80:12 (browser_tests+0x408fed0)
    #15 blink::scheduler::internal::TaskQueueImpl::OnTaskCompleted(blink::scheduler::TaskQueue::Task const&, base::TimeTicks, base::TimeTicks) third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:913 (browser_tests+0x408fed0)
    #16 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue(blink::scheduler::internal::WorkQueue*, bool, blink::scheduler::LazyNow, base::TimeTicks*) third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:556:12 (browser_tests+0x409756d)
    #17 blink::scheduler::TaskQueueManager::DoWork(bool) third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:330:13 (browser_tests+0x40939f5)
    #18 Invoke<const base::WeakPtr<blink::scheduler::TaskQueueManager> &, const bool &> base/bind_internal.h:196:12 (browser_tests+0x409a027)
    #19 void base::internal::InvokeHelper<true, void>::MakeItSo<void (blink::scheduler::TaskQueueManager::* const&)(bool), base::WeakPtr<blink::scheduler::TaskQueueManager> const&, bool const&>(void (blink::scheduler::TaskQueueManager::* const&)(bool), base::WeakPtr<blink::scheduler::TaskQueueManager> const&, bool const&) base/bind_internal.h:285 (browser_tests+0x409a027)
    #20 RunImpl<void (blink::scheduler::TaskQueueManager::*const &)(bool), const std::__1::tuple<base::WeakPtr<blink::scheduler::TaskQueueManager>, bool> &, 0, 1> base/bind_internal.h:340:12 (browser_tests+0x4099ee2)
    #21 base::internal::Invoker<base::internal::BindState<void (blink::scheduler::TaskQueueManager::*)(bool), base::WeakPtr<blink::scheduler::TaskQueueManager>, bool>, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:319 (browser_tests+0x4099ee2)
    #22 Run base/callback.h:91:12 (browser_tests+0x42a17aa)
    #23 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:59 (browser_tests+0x42a17aa)
    #24 base::MessageLoop::RunTask(base::PendingTask*) base/message_loop/message_loop.cc:403:19 (browser_tests+0x42daa1f)
    #25 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) base/message_loop/message_loop.cc:414:5 (browser_tests+0x42daf0b)
    #26 base::MessageLoop::DoWork() base/message_loop/message_loop.cc:521:13 (browser_tests+0x42db3bb)
    #27 base::MessagePumpDefault::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_default.cc:33:31 (browser_tests+0x42def5a)
    #28 base::MessageLoop::Run() base/message_loop/message_loop.cc:350:10 (browser_tests+0x42da373)
    #29 non-virtual thunk to base::MessageLoop::Run() base/message_loop/message_loop.cc (browser_tests+0x42da3bd)
    #30 base::RunLoop::Run() base/run_loop.cc:111:14 (browser_tests+0x43232a4)
    #31 content::RendererMain(content::MainFunctionParams const&) content/renderer/renderer_main.cc:219:23 (browser_tests+0xaddd4c1)
    #32 content::RunZygote(content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:337:14 (browser_tests+0x4262f69)
    #33 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:416:12 (browser_tests+0x4263bfd)
    #34 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:687:12 (browser_tests+0x4264917)
    #35 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() content/app/content_service_manager_main_delegate.cc:51:32 (browser_tests+0x42623cf)
    #36 service_manager::Main(service_manager::MainParams const&) services/service_manager/embedder/main.cc:469:29 (browser_tests+0x6f87e0a)
    #37 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:10 (browser_tests+0x4262b6b)
    #38 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:526:12 (browser_tests+0x4ca8817)
    #39 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:149:10 (browser_tests+0x428b700)
    #40 main chrome/test/base/browser_tests_main.cc:21:10 (browser_tests+0x428b03a)
  Previous write of size 1 at 0x000010694ee0 by thread T6:
    #0 base::PersistentHistogramAllocator::GetCreateHistogramResultHistogram() base/metrics/persistent_histogram_allocator.cc (browser_tests+0x42f943b)
    #1 RecordCreateHistogramResult base/metrics/persistent_histogram_allocator.cc:741:37 (browser_tests+0x42f802a)
    #2 base::PersistentHistogramAllocator::CreateHistogram(base::PersistentHistogramAllocator::PersistentHistogramData*) base/metrics/persistent_histogram_allocator.cc:697 (browser_tests+0x42f802a)
    #3 base::PersistentHistogramAllocator::AllocateHistogram(base::HistogramType, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, int, base::BucketRanges const*, int, unsigned int*) base/metrics/persistent_histogram_allocator.cc:412:48 (browser_tests+0x42f888e)
    #4 base::Histogram::Factory::Build() base/metrics/histogram.cc:186:40 (browser_tests+0x42eddf4)
    #5 base::LinearHistogram::FactoryGetWithRangeDescription(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, int, unsigned int, int, base::LinearHistogram::DescriptionPair const*) base/metrics/histogram.cc:939:8 (browser_tests+0x42f1a47)
    #6 base::LinearHistogram::FactoryGet(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, int, unsigned int, int) base/metrics/histogram.cc:883:10 (browser_tests+0x42f190c)
    #7 base::PersistentMemoryAllocator::CreateTrackingHistograms(base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >) base/metrics/persistent_memory_allocator.cc:479:21 (browser_tests+0x42fc833)
    #8 base::PersistentHistogramAllocator::CreateTrackingHistograms(base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >) base/metrics/persistent_histogram_allocator.cc:517:22 (browser_tests+0x42f9383)
    #9 content::ChildHistogramMessageFilter::OnSetHistogramMemory(base::SharedMemoryHandle const&, int) content/child/child_histogram_message_filter.cc:60:23 (browser_tests+0x8799373)
    #10 DispatchToMethodImpl<content::ChildHistogramMessageFilter *, void (content::ChildHistogramMessageFilter::*)(const base::SharedMemoryHandle &, int), const std::__1::tuple<base::SharedMemoryHandle, int> &, 0, 1> base/tuple.h:77:3 (browser_tests+0x87991e2)
    #11 DispatchToMethod<content::ChildHistogramMessageFilter *, void (content::ChildHistogramMessageFilter::*)(const base::SharedMemoryHandle &, int), const std::__1::tuple<base::SharedMemoryHandle, int> &> base/tuple.h:84 (browser_tests+0x87991e2)
    #12 DispatchToMethod<content::ChildHistogramMessageFilter, void (content::ChildHistogramMessageFilter::*)(const base::SharedMemoryHandle &, int), void, std::__1::tuple<base::SharedMemoryHandle, int> > ipc/ipc_message_templates.h:26 (browser_tests+0x87991e2)
    #13 bool IPC::MessageT<ChildProcessMsg_SetHistogramMemory_Meta, std::__1::tuple<base::SharedMemoryHandle, int>, void>::Dispatch<content::ChildHistogramMessageFilter, content::ChildHistogramMessageFilter, void, void (content::ChildHistogramMessageFilter::*)(base::SharedMemoryHandle const&, int)>(IPC::Message const*, content::ChildHistogramMessageFilter*, content::ChildHistogramMessageFilter*, void*, void (content::ChildHistogramMessageFilter::*)(base::SharedMemoryHandle const&, int)) ipc/ipc_message_templates.h:121 (browser_tests+0x87991e2)
    #14 content::ChildHistogramMessageFilter::OnMessageReceived(IPC::Message const&) content/child/child_histogram_message_filter.cc:40:5 (browser_tests+0x8798fc0)
    #15 TryFiltersImpl ipc/message_filter_router.cc:22:21 (browser_tests+0x61eae24)
    #16 IPC::MessageFilterRouter::TryFilters(IPC::Message const&) ipc/message_filter_router.cc:80 (browser_tests+0x61eae24)
    #17 IPC::ChannelProxy::Context::TryFilters(IPC::Message const&) ipc/ipc_channel_proxy.cc:87:31 (browser_tests+0x61cc22d)
    #18 IPC::SyncChannel::SyncContext::OnMessageReceived(IPC::Message const&) ipc/ipc_sync_channel.cc:466:7 (browser_tests+0x61e261a)
    #19 IPC::ChannelMojo::OnMessageReceived(IPC::Message const&) ipc/ipc_channel_mojo.cc:414:14 (browser_tests+0x61c6f9f)
    #20 non-virtual thunk to IPC::ChannelMojo::OnMessageReceived(IPC::Message const&) ipc/ipc_channel_mojo.cc (browser_tests+0x61c72c4)
    #21 IPC::internal::MessagePipeReader::Receive(std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > const&, base::Optional<std::__1::vector<mojo::StructPtr<IPC::mojom::SerializedHandle>, std::__1::allocator<mojo::StructPtr<IPC::mojom::SerializedHandle> > > >) ipc/ipc_message_pipe_reader.cc:109:14 (browser_tests+0x61d3795)
    #22 IPC::mojom::ChannelStubDispatch::Accept(IPC::mojom::Channel*, mojo::Message*) out/Release/gen/ipc/ipc.mojom.cc:269:13 (browser_tests+0x61ec7db)
    #23 IPC::mojom::ChannelStub<mojo::RawPtrImplRefTraits<IPC::mojom::Channel> >::Accept(mojo::Message*) out/Release/gen/ipc/ipc.mojom.h:276:12 (browser_tests+0x61d3bcf)
    #24 mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:420:32 (browser_tests+0x6197303)
    #25 mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept(mojo::Message*) mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:132:18 (browser_tests+0x6196d3a)
    #26 mojo::FilterChain::Accept(mojo::Message*) mojo/public/cpp/bindings/lib/filter_chain.cc:40:17 (browser_tests+0x61ba87a)
    #27 mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:307:19 (browser_tests+0x619908a)
    #28 IPC::(anonymous namespace)::ChannelAssociatedGroupController::Accept(mojo::Message*) ipc/ipc_mojo_bootstrap.cc:753:20 (browser_tests+0x61d8d88)
    #29 non-virtual thunk to IPC::(anonymous namespace)::ChannelAssociatedGroupController::Accept(mojo::Message*) ipc/ipc_mojo_bootstrap.cc (browser_tests+0x61daac4)
    #30 mojo::FilterChain::Accept(mojo::Message*) mojo/public/cpp/bindings/lib/filter_chain.cc:40:17 (browser_tests+0x61ba87a)
    #31 mojo::Connector::ReadSingleMessage(unsigned int*) mojo/public/cpp/bindings/lib/connector.cc:440:51 (browser_tests+0x6194e44)
    #32 mojo::Connector::ReadAllAvailableMessages() mojo/public/cpp/bindings/lib/connector.cc:469:10 (browser_tests+0x6195d4a)
    #33 mojo::Connector::OnHandleReadyInternal(unsigned int) mojo/public/cpp/bindings/lib/connector.cc:374:3 (browser_tests+0x6195afe)
    #34 mojo::Connector::OnWatcherHandleReady(unsigned int) mojo/public/cpp/bindings/lib/connector.cc:351:3 (browser_tests+0x6195a50)
    #35 Invoke<mojo::Connector *, unsigned int> base/bind_internal.h:196:12 (browser_tests+0x61967f4)
    #36 MakeItSo<void (mojo::Connector::*const &)(unsigned int), mojo::Connector *, unsigned int> base/bind_internal.h:265 (browser_tests+0x61967f4)
    #37 RunImpl<void (mojo::Connector::*const &)(unsigned int), const std::__1::tuple<base::internal::UnretainedWrapper<mojo::Connector> > &, 0> base/bind_internal.h:340 (browser_tests+0x61967f4)
    #38 base::internal::Invoker<base::internal::BindState<void (mojo::Connector::*)(unsigned int), base::internal::UnretainedWrapper<mojo::Connector> >, void (unsigned int)>::Run(base::internal::BindStateBase*, unsigned int&&) base/bind_internal.h:319 (browser_tests+0x61967f4)
    #39 Run base/callback.h:80:12 (browser_tests+0x1fc5cd5)
    #40 mojo::SimpleWatcher::DiscardReadyState(base::Callback<void (unsigned int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned int, mojo::HandleSignalsState const&) mojo/public/cpp/system/simple_watcher.h:193 (browser_tests+0x1fc5cd5)
    #41 Invoke<const base::Callback<void (unsigned int), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> &, unsigned int, const mojo::HandleSignalsState &> base/bind_internal.h:151:12 (browser_tests+0x1fc5d31)
    #42 MakeItSo<void (*const &)(const base::Callback<void (unsigned int), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> &, unsigned int, const mojo::HandleSignalsState &), const base::Callback<void (unsigned int), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> &, unsigned int, const mojo::HandleSignalsState &> base/bind_internal.h:265 (browser_tests+0x1fc5d31)
    #43 RunImpl<void (*const &)(const base::Callback<void (unsigned int), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> &, unsigned int, const mojo::HandleSignalsState &), const std::__1::tuple<base::Callback<void (unsigned int), base::internal::CopyMode::Copyable, base::internal::RepeatMode::Repeating> > &, 0> base/bind_internal.h:340 (browser_tests+0x1fc5d31)
    #44 base::internal::Invoker<base::internal::BindState<void (*)(base::Callback<void (unsigned int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned int, mojo::HandleSignalsState const&), base::Callback<void (unsigned int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> >, void (unsigned int, mojo::HandleSignalsState const&)>::Run(base::internal::BindStateBase*, unsigned int&&, mojo::HandleSignalsState const&) base/bind_internal.h:319 (browser_tests+0x1fc5d31)
    #45 Run base/callback.h:80:12 (browser_tests+0x61bf804)
    #46 mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&) mojo/public/cpp/system/simple_watcher.cc:276 (browser_tests+0x61bf804)
    #47 Invoke<const base::WeakPtr<mojo::SimpleWatcher> &, const int &, const unsigned int &, const mojo::HandleSignalsState &> base/bind_internal.h:196:12 (browser_tests+0x61bff5c)
    #48 void base::internal::InvokeHelper<true, void>::MakeItSo<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher> const&, int const&, unsigned int const&, mojo::HandleSignalsState const&>(void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher> const&, int const&, unsigned int const&, mojo::HandleSignalsState const&) base/bind_internal.h:285 (browser_tests+0x61bff5c)
    #49 RunImpl<void (mojo::SimpleWatcher::*const &)(int, unsigned int, const mojo::HandleSignalsState &), const std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> &, 0, 1, 2, 3> base/bind_internal.h:340:12 (browser_tests+0x61bfdea)
    #50 base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:319 (browser_tests+0x61bfdea)
    #51 Run base/callback.h:91:12 (browser_tests+0x42a17aa)
    #52 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:59 (browser_tests+0x42a17aa)
    #53 base::MessageLoop::RunTask(base::PendingTask*) base/message_loop/message_loop.cc:403:19 (browser_tests+0x42daa1f)
    #54 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) base/message_loop/message_loop.cc:414:5 (browser_tests+0x42daf0b)
    #55 base::MessageLoop::DoWork() base/message_loop/message_loop.cc:521:13 (browser_tests+0x42db3bb)
    #56 base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_libevent.cc:220:31 (browser_tests+0x42e1210)
    #57 base::MessageLoop::Run() base/message_loop/message_loop.cc:350:10 (browser_tests+0x42da373)
    #58 non-virtual thunk to base::MessageLoop::Run() base/message_loop/message_loop.cc (browser_tests+0x42da3bd)
    #59 base::RunLoop::Run() base/run_loop.cc:111:14 (browser_tests+0x43232a4)
    #60 base::Thread::Run(base::RunLoop*) base/threading/thread.cc:255:13 (browser_tests+0x436bc1b)
    #61 base::Thread::ThreadMain() base/threading/thread.cc:338:3 (browser_tests+0x436c3b1)
    #62 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:71:13 (browser_tests+0x435fef8)
  Location is global 'base::PersistentHistogramAllocator::GetCreateHistogramResultHistogram()::initialized' of size 1 at 0x000010694ee0 (browser_tests+0x000010694ee0)
  Thread T6 'Chrome_ChildIOThread' (tid=25183, running) created by main thread at:
    #0 pthread_create <null> (browser_tests+0x58fbe3)
    #1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:110:13 (browser_tests+0x435f836)
    #2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:193:10 (browser_tests+0x435f6f5)
    #3 base::Thread::StartWithOptions(base::Thread::Options const&) base/threading/thread.cc:112:15 (browser_tests+0x436b223)
    #4 content::ChildProcess::ChildProcess(base::ThreadPriority, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::unique_ptr<base::TaskScheduler::InitParams, std::__1::default_delete<base::TaskScheduler::InitParams> >) content/child/child_process.cc:76:3 (browser_tests+0x8747bbc)
    #5 content::RenderProcess::RenderProcess(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::unique_ptr<base::TaskScheduler::InitParams, std::__1::default_delete<base::TaskScheduler::InitParams> >) content/renderer/render_process.cc:14:7 (browser_tests+0xafe8412)
    #6 content::RenderProcessImpl::RenderProcessImpl(std::__1::unique_ptr<base::TaskScheduler::InitParams, std::__1::default_delete<base::TaskScheduler::InitParams> >) content/renderer/render_process_impl.cc:106:7 (browser_tests+0xad89883)
    #7 content::RenderProcessImpl::Create() content/renderer/render_process_impl.cc:189:11 (browser_tests+0xad89dce)
    #8 content::RendererMain(content::MainFunctionParams const&) content/renderer/renderer_main.cc:206:27 (browser_tests+0xaddd401)
    #9 content::RunZygote(content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:337:14 (browser_tests+0x4262f69)
    #10 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:416:12 (browser_tests+0x4263bfd)
    #11 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:687:12 (browser_tests+0x4264917)
    #12 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() content/app/content_service_manager_main_delegate.cc:51:32 (browser_tests+0x42623cf)
    #13 service_manager::Main(service_manager::MainParams const&) services/service_manager/embedder/main.cc:469:29 (browser_tests+0x6f87e0a)
    #14 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:10 (browser_tests+0x4262b6b)
    #15 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:526:12 (browser_tests+0x4ca8817)
    #16 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:149:10 (browser_tests+0x428b700)
    #17 main chrome/test/base/browser_tests_main.cc:21:10 (browser_tests+0x428b03a)
SUMMARY: ThreadSanitizer: data race base/metrics/persistent_histogram_allocator.cc in base::PersistentHistogramAllocator::GetCreateHistogramResultHistogram()
 

Comment 1 by gab@chromium.org, Aug 7 2017

Blocking: 368525
Labels: -Pri-3 M-62 Pri-1
Owner: bcwh...@chromium.org
Status: Assigned (was: Asss)
The first histograms are normally created when the process is still single-threaded after which it is initialized and the histogram_pointer is valid.  As such, only atomic ops will be used by this method once there are multiple threads.

You're right, though, that it should be a thread-local flag.  I think I can do it better, though, by having a "under construction" flag inside the existing atomic pointer word.

Comment 3 by gab@chromium.org, Aug 7 2017

1) hmmm, well TSan tripped on a case where it's not initialized while single-threaded.

2) "under construction" flag sounds a *lot* like LazyInstance's impl, why not just use that?
I checked the LazyInstance code and used that as the basis of my implementation.  The LazyInstance class, however, has the ctor hard-coded as the "creator_func" and I have to store the result of FactoryGet().

Taking another look, though...  Perhaps all I have to do is define the Traits appropriately.  Let me give that a try.
LazyInstance reserves internal space for the object of the defined type which will go unused.  We'll be wasting 40 bytes or so but seems a small price for reusing standard code.  Okay?

Comment 6 by gab@chromium.org, Aug 7 2017

Seems worth it to me, yes.

Which field is wasting 40 bytes?
It would reserve space for a HistogramBase object.  That's currently 36 bytes on 32-bit build though I hope to reduce it to about 16.

The space will go unused because FactoryGet() already creates the object on the heap.

But I just remembered the other reason I abandoned trying to use LazyInstance...  If it is "under construction", it LazyInstance yields the CPU until construction is finished.  I need to return nullptr because this method has to deal with being called recursively.
Components: Internals>Metrics
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 14 2017

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

commit 1b460e69a28391f0590f61f6ebf5c341d1e7df23
Author: Brian White <bcwhite@chromium.org>
Date: Mon Aug 14 23:24:21 2017

Use internal flag to protect against multiple initialization of histogram.

Bug:  753030 
Change-Id: I473efe6ff97fdfd6040b548a1db2f7d31870775e
Reviewed-on: https://chromium-review.googlesource.com/604490
Reviewed-by: Alexei Svitkine (very slow) <asvitkine@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494239}
[modify] https://crrev.com/1b460e69a28391f0590f61f6ebf5c341d1e7df23/base/metrics/persistent_histogram_allocator.cc

Status: Fixed (was: Assigned)

Sign in to add a comment