Fix invalidation of TagKeys. (#401)

Allocate name strings separately so that references to them are stable
even if the underlying storage grows / moves.

Add test that fails if this isn't done.

Author: g-easy

opencensus/tags/internal/tag_key.cc

@@ -15,12 +15,14 @@
 #include "opencensus/tags/tag_key.h"
 
 #include <cstdint>
+#include <memory>
 #include <string>
 #include <unordered_map>
 #include <utility>
 #include <vector>
 
 #include "absl/base/thread_annotations.h"
+#include "absl/memory/memory.h"
 #include "absl/strings/string_view.h"
 #include "absl/synchronization/mutex.h"
 
@@ -38,13 +40,16 @@ class TagKeyRegistry {
 
   const std::string& TagKeyName(TagKey key) const LOCKS_EXCLUDED(mu_) {
     absl::ReaderMutexLock l(&mu_);
-    return registered_tag_keys_[key.id_];
+    return *registered_tag_keys_[key.id_];
   }
 
  private:
   mutable absl::Mutex mu_;
-  // The registered tag keys. Tag key ids are indices into this vector.
-  std::vector<std::string> registered_tag_keys_ GUARDED_BY(mu_);
+  // The registered tag keys. Tag key ids are indices into this vector.  Strings
+  // are allocated individually so that they don't move around when the vector
+  // storage moves due to resize.
+  std::vector<std::unique_ptr<std::string>> registered_tag_keys_
+      GUARDED_BY(mu_);
   // A map from names to IDs.
   // TODO: change to string_view when a suitable hash is available.
   std::unordered_map<std::string, uint64_t> id_map_ GUARDED_BY(mu_);
@@ -56,7 +61,7 @@ TagKey TagKeyRegistry::Register(absl::string_view name) {
   const auto it = id_map_.find(string_name);
   if (it == id_map_.end()) {
     const uint64_t id = registered_tag_keys_.size();
-    registered_tag_keys_.emplace_back(name);
+    registered_tag_keys_.emplace_back(absl::make_unique<std::string>(name));
     id_map_.emplace_hint(it, string_name, id);
     return TagKey(id);
   }

opencensus/tags/internal/tag_key_test.cc

@@ -15,6 +15,7 @@
 #include "opencensus/tags/tag_key.h"
 
 #include "absl/hash/hash.h"
+#include "absl/strings/str_cat.h"
 #include "gtest/gtest.h"
 
 namespace opencensus {
@@ -40,6 +41,18 @@ TEST(TagKeyTest, Inequality) {
   EXPECT_NE(absl::Hash<TagKey>()(k1), absl::Hash<TagKey>()(k2));
 }
 
+// Test that the reference returned by TagKey::name() isn't invalidated when the
+// registry's underlying storage grows and moves.
+TEST(TagKeyTest, GrowRegistry) {
+  TagKey k = TagKey::Register("my_key");
+  const std::string& s = k.name();
+  ASSERT_EQ("my_key", s);
+  for (int i = 0; i < 1000; ++i) {
+    TagKey::Register(absl::StrCat("key_", i));
+    EXPECT_EQ("my_key", s) << "iteration " << i;
+  }
+}
+
 }  // namespace
 }  // namespace tags
 }  // namespace opencensus