backport to 1.16: aggregate cluster: fix TLS init issue (#14456)

Additional Description: Based on #14388 Risk Level: Low Testing: Build and run the repro from #14119 without crashing, `bazel test test/extensions/clusters/aggregate:cluster_test` Docs Changes: N/A Release Notes: #14119 Signed-off-by: Taylor Barrella <tabarr@google.com>
envoyproxy · Jan 15, 2021 · 08c9c49 · 08c9c49
1 parent 15f02f0
commit 08c9c49
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 2 deletions.
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -13,6 +13,7 @@ Bug Fixes
 ---------
 *Changes expected to improve the state of the world and are unlikely to have negative effects*
 
+* aggregate cluster: fixed a crash due to a TLS initialization issue.
 * lua: fixed crash when Lua script contains streamInfo():downstreamSslConnection().
 * tls: fix detection of the upstream connection close event.
 

diff --git a/source/extensions/clusters/aggregate/cluster.cc b/source/extensions/clusters/aggregate/cluster.cc
@@ -20,7 +20,9 @@ Cluster::Cluster(const envoy::config::cluster::v3::Cluster& cluster,
     : Upstream::ClusterImplBase(cluster, runtime, factory_context, std::move(stats_scope),
                                 added_via_api),
       cluster_manager_(cluster_manager), runtime_(runtime), random_(random),
-      tls_(tls.allocateSlot()), clusters_(config.clusters().begin(), config.clusters().end()) {}
+      tls_(tls.allocateSlot()), clusters_(config.clusters().begin(), config.clusters().end()) {
+  tls_->set([](Event::Dispatcher&) { return nullptr; });
+}
 
 PriorityContextPtr
 Cluster::linearizePrioritySet(const std::function<bool(const std::string&)>& skip_predicate) {

diff --git a/test/mocks/thread_local/mocks.h b/test/mocks/thread_local/mocks.h
@@ -58,17 +58,23 @@ class MockInstance : public Instance {
     }
 
     // ThreadLocal::Slot
-    ThreadLocalObjectSharedPtr get() override { return parent_.data_[index_]; }
+    ThreadLocalObjectSharedPtr get() override {
+      EXPECT_TRUE(was_set_);
+      return parent_.data_[index_];
+    }
     bool currentThreadRegistered() override { return parent_.registered_; }
     void runOnAllThreads(const UpdateCb& cb) override {
+      EXPECT_TRUE(was_set_);
       parent_.runOnAllThreads([cb, this]() { parent_.data_[index_] = cb(parent_.data_[index_]); });
     }
     void runOnAllThreads(const UpdateCb& cb, Event::PostCb main_callback) override {
+      EXPECT_TRUE(was_set_);
       parent_.runOnAllThreads([cb, this]() { parent_.data_[index_] = cb(parent_.data_[index_]); },
                               main_callback);
     }
 
     void set(InitializeCb cb) override {
+      was_set_ = true;
       if (parent_.defer_data) {
         parent_.deferred_data_[index_] = cb;
       } else {
@@ -78,6 +84,7 @@ class MockInstance : public Instance {
 
     MockInstance& parent_;
     const uint32_t index_;
+    bool was_set_{}; // set() must be called before other functions.
   };
 
   void call() {