Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1189)

Unified Diff: base/metrics/field_trial.cc

Issue 2511413002: Add debug info for field trial allocation crash (Closed)
Patch Set: iterate through allocator to check for existing allocator Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/metrics/field_trial.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/metrics/field_trial.cc
diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index 970441160ca4afe2acb501095c8e81716e3c45ce..784ea7439754e713fd77b073296f3a9a07f3f785 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -181,6 +181,48 @@ void AddForceFieldTrialsFlag(CommandLine* cmd_line) {
}
}
+// In order to debug the crash where two field trials with the same name but
+// different groups get added to the allocator, let's try to get the stack
+// frame where the second duplicate gets added here as well as the trial/group
+// names by going through the allocator and checking if a trial with the same
+// name already exists.
+void CheckAllocatorForTrial(FieldTrialList::FieldTrialAllocator* allocator,
+ std::string trial_name,
Alexei Svitkine (slow) 2016/11/21 20:11:33 Nit: const& Same for the next param.
lawrencewu 2016/11/21 20:41:23 Done.
+ std::string group_name) {
+ FieldTrialList::FieldTrialAllocator::Iterator iter(allocator);
+ FieldTrial::FieldTrialRef ref;
+ while ((ref = iter.GetNextOfType(kFieldTrialType)) !=
+ FieldTrialList::FieldTrialAllocator::kReferenceNull) {
+ const FieldTrialEntry* entry =
+ allocator->GetAsObject<const FieldTrialEntry>(ref, kFieldTrialType);
+
+ StringPiece trial_name_to_check;
+ StringPiece group_name_to_check;
+ if (!entry->GetTrialAndGroupName(&trial_name_to_check,
+ &group_name_to_check))
Alexei Svitkine (slow) 2016/11/21 20:11:33 Nit: {}'s
lawrencewu 2016/11/21 20:41:23 Done.
+ CHECK(false);
+
+ std::string existing_trial_name = trial_name_to_check.as_string();
+ std::string existing_group_name = group_name_to_check.as_string();
+ if (existing_trial_name == trial_name) {
+ constexpr size_t buf_size = 100;
+
+ char trial_name_c_str[buf_size];
+ char group_name_c_str[buf_size];
+ char existing_group_name_c_str[buf_size];
+
+ strlcpy(trial_name_c_str, trial_name.c_str(), buf_size);
+ strlcpy(group_name_c_str, group_name.c_str(), buf_size);
+ strlcpy(existing_group_name_c_str, existing_group_name.c_str(), buf_size);
+
+ debug::Alias(trial_name_c_str);
+ debug::Alias(existing_group_name_c_str);
+ debug::Alias(group_name_c_str);
+ CHECK_EQ(existing_group_name, group_name);
+ }
+ }
+}
+
#if defined(OS_WIN)
HANDLE CreateReadOnlyHandle(FieldTrialList::FieldTrialAllocator* allocator) {
HANDLE src = allocator->shared_memory()->handle().GetHandle();
@@ -857,14 +899,30 @@ bool FieldTrialList::CreateTrialsFromSharedMemory(
// If we failed to create the field trial, crash with debug info.
// TODO(665129): Remove this when the crash is resolved.
if (!trial) {
+ constexpr size_t buf_size = 100;
+ char trial_name_c_str[buf_size] = {0};
+ char group_name_c_str[buf_size] = {0};
+ char existing_group_name_c_str[buf_size] = {0};
+
+ // Copy the names over to the stack.
std::string trial_name_string = trial_name.as_string();
+ strlcpy(trial_name_c_str, trial_name_string.c_str(), buf_size);
std::string group_name_string = group_name.as_string();
+ strlcpy(group_name_c_str, group_name_string.c_str(), buf_size);
+
+ // Alias the trial and group name.
+ debug::Alias(trial_name_c_str);
+ debug::Alias(group_name_c_str);
+
+ // Copy and alias the existing field trial name, if there is one.
FieldTrial* existing_field_trial =
FieldTrialList::Find(trial_name_string);
- if (existing_field_trial)
- debug::Alias(existing_field_trial->group_name_internal().c_str());
- debug::Alias(trial_name_string.c_str());
- debug::Alias(group_name_string.c_str());
+ if (existing_field_trial) {
+ std::string existing_group_name_string =
+ existing_field_trial->group_name_internal();
+ strlcpy(existing_group_name_c_str, group_name_string.c_str(), buf_size);
+ debug::Alias(existing_group_name_c_str);
+ }
CHECK(!trial_name_string.empty());
CHECK(!group_name_string.empty());
CHECK_EQ(existing_field_trial->group_name_internal(),
@@ -919,6 +977,10 @@ void FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded() {
void FieldTrialList::AddToAllocatorWhileLocked(FieldTrial* field_trial) {
FieldTrialAllocator* allocator = global_->field_trial_allocator_.get();
+ // TODO(665129): Remove this code once the bug has been fixed.
+ CheckAllocatorForTrial(allocator, field_trial->trial_name(),
+ field_trial->group_name_internal());
+
// Don't do anything if the allocator hasn't been instantiated yet.
if (allocator == nullptr)
return;
« no previous file with comments | « base/metrics/field_trial.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698