New issue
Advanced search Search tips

Issue 603028 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Cronet's HistogramManager isn't thread-safe

Project Member Reported by pauljensen@chromium.org, Apr 13 2016

Issue description

Version: M51
OS: Android

What steps will reproduce the problem?
Access CronetEngine.getGlobalMetricsDeltas() from multiple CronetEngine's on multiple threads.

What is the expected output?
No crash.

What do you see instead?
Crash, with call stack like:
  base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::BasicStringPiece(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) at :?
  metrics::EncodeHistogramDelta(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::HistogramSamples const&, metrics::ChromeUserMetricsExtension*) at :?
  base::HistogramSnapshotManager::FinishDeltas() at :?
  cronet::HistogramManager::GetDeltas(std::__1::vector<unsigned char, std::__1::allocator<unsigned char> >*) at :?
  Java_org_chromium_net_CronetUrlRequestContext_nativeGetHistogramDeltas at :?

Internal bug b/27553604
 
Status: Started (was: Assigned)
Fix in progress: https://codereview.chromium.org/1882563004
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 13 2016

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

commit 487de2df8db4a96fd24e6998590ae715142447ec
Author: pauljensen <pauljensen@chromium.org>
Date: Wed Apr 13 17:05:37 2016

[Cronet] Make HistogramManager public APIs thread-safe.

CronetEngine.getGlobalMetricsDeltas() exposes the underlying
HistogramManager public APIs on multiple threads, so crashes
can occur if accessed concurrently. The current public APIs
are GetInstance() which simply accesses a thread-safe
LazyInstance, and GetDeltas() which doesn't look thread-safe
so I've added a Lock.

BUG= 603028 

Review URL: https://codereview.chromium.org/1882563004

Cr-Commit-Position: refs/heads/master@{#387009}

[modify] https://crrev.com/487de2df8db4a96fd24e6998590ae715142447ec/components/cronet/histogram_manager.cc
[modify] https://crrev.com/487de2df8db4a96fd24e6998590ae715142447ec/components/cronet/histogram_manager.h

Status: Fixed (was: Started)
Issue 597022 has been merged into this issue.

Sign in to add a comment