From 6f6d2b924aa9a82a00cb8923b4bbf0c8f9a283b1 Mon Sep 17 00:00:00 2001
From: John Plevyak <jplevyak@gmail.com>
Date: Fri, 8 Feb 2019 17:17:54 -0800
Subject: [PATCH 1/6] Fix lifetime issue which was causing non-clonable (i.e.
 V8) WAVM VMs to fail.

---
 include/envoy/server/wasm_config.h            |  6 ++
 .../extensions/access_loggers/wasm/config.cc  | 31 +++++-----
 source/extensions/common/wasm/wasm.cc         | 61 +++++++++++++------
 source/extensions/common/wasm/wasm.h          | 47 ++++++++------
 .../filters/http/wasm/wasm_filter.cc          | 30 ++++-----
 source/extensions/wasm/config.cc              | 22 ++++---
 source/server/server.cc                       |  4 +-
 source/server/wasm_config_impl.h              |  8 ++-
 8 files changed, 128 insertions(+), 81 deletions(-)

diff --git a/include/envoy/server/wasm_config.h b/include/envoy/server/wasm_config.h
index 07d91a0d0d4d1..c8520c4954bec 100644
--- a/include/envoy/server/wasm_config.h
+++ b/include/envoy/server/wasm_config.h
@@ -5,6 +5,7 @@
 #include "envoy/event/dispatcher.h"
 #include "envoy/server/wasm.h"
 #include "envoy/thread_local/thread_local.h"
+#include "envoy/upstream/cluster_manager.h"
 
 #include "common/protobuf/protobuf.h"
 
@@ -16,6 +17,11 @@ class WasmFactoryContext {
 public:
   virtual ~WasmFactoryContext() {}
 
+  /**
+   * @return Upstream::ClusterManager& singleton for use by the entire server.
+   */
+  virtual Upstream::ClusterManager& clusterManager() PURE;
+
   /**
    * @return Event::Dispatcher& the main thread's dispatcher. This dispatcher should be used
    *         for all singleton processing.
diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc
index 959f93f6f333b..13f640c8fc172 100644
--- a/source/extensions/access_loggers/wasm/config.cc
+++ b/source/extensions/access_loggers/wasm/config.cc
@@ -20,30 +20,29 @@ namespace AccessLoggers {
 namespace Wasm {
 
 AccessLog::InstanceSharedPtr
-WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config,
+WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_config,
                                               AccessLog::FilterPtr&& filter,
                                               Server::Configuration::FactoryContext& context) {
-  const auto& wal_config =
-      MessageUtil::downcastAndValidate<const envoy::config::accesslog::v2::WasmAccessLog&>(config);
+  const auto& config =
+      MessageUtil::downcastAndValidate<const envoy::config::accesslog::v2::WasmAccessLog&>(
+          proto_config);
+  auto id = config.id();
+  auto configuration = std::make_shared<std::string>(config.configuration());
   auto tls_slot = context.threadLocal().allocateSlot();
-  if (wal_config.has_vm_config()) {
-    auto base_wasm =
-        Common::Wasm::createWasm(wal_config.id(), wal_config.vm_config(), context.api());
-    tls_slot->set([&wal_config, &base_wasm, &context](Event::Dispatcher& dispatcher) {
-      auto result =
-          Common::Wasm::createThreadLocalWasm(*base_wasm, wal_config.vm_config(), dispatcher,
-                                              wal_config.configuration(), context.api());
-      result->setClusterManager(context.clusterManager());
+  if (config.has_vm_config()) {
+    auto base_wasm = Common::Wasm::createWasm(id, config.vm_config(), context.clusterManager(),
+                                              context.dispatcher(), context.api());
+    // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.
+    tls_slot->set([base_wasm, configuration](Event::Dispatcher& dispatcher) {
+      auto result = Common::Wasm::createThreadLocalWasm(*base_wasm, *configuration, dispatcher);
       return std::static_pointer_cast<ThreadLocal::ThreadLocalObject>(result);
     });
   } else {
-    if (wal_config.id().empty()) {
+    if (id.empty()) {
       throw Common::Wasm::WasmVmException("No WASM VM Id or vm_config specified");
     }
-    tls_slot->set([&wal_config, &context](Event::Dispatcher& dispatcher) {
-      auto result = Common::Wasm::getThreadLocalWasm(wal_config.id(), wal_config.configuration());
-      result->setDispatcher(dispatcher);
-      result->setClusterManager(context.clusterManager());
+    tls_slot->set([id, configuration](Event::Dispatcher&) {
+      auto result = Common::Wasm::getThreadLocalWasm(id, *configuration);
       return std::static_pointer_cast<ThreadLocal::ThreadLocalObject>(result);
     });
   }
diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc
index e257f9fadf927..5860bac642a0b 100644
--- a/source/extensions/common/wasm/wasm.cc
+++ b/source/extensions/common/wasm/wasm.cc
@@ -640,10 +640,7 @@ uint32_t Context::httpCall(absl::string_view cluster, const Pairs& request_heade
   if (timeout_milliseconds < 0)
     return 0;
   auto cluster_string = std::string(cluster);
-  auto cluster_manager = clusterManager();
-  if (!cluster_manager)
-    return 0;
-  if (cluster_manager->get(cluster_string) == nullptr)
+  if (clusterManager().get(cluster_string) == nullptr)
     return 0;
 
   Http::MessagePtr message(new Http::RequestMessageImpl(buildHeaderMapFromPairs(request_headers)));
@@ -670,7 +667,8 @@ uint32_t Context::httpCall(absl::string_view cluster, const Pairs& request_heade
 
   auto token = next_async_token_++;
   auto& handler = http_request_[token];
-  auto http_request = cluster_manager->httpAsyncClientForCluster(cluster_string)
+  auto http_request = clusterManager()
+                          .httpAsyncClientForCluster(cluster_string)
                           .send(std::move(message), handler,
                                 Http::AsyncClient::RequestOptions().setTimeout(timeout));
   if (!http_request) {
@@ -932,7 +930,10 @@ void Context::onHttpCallResponse(uint32_t token, const Pairs& response_headers,
                              trailers_ptr, trailers_size);
 }
 
-Wasm::Wasm(absl::string_view vm, absl::string_view id) {
+Wasm::Wasm(absl::string_view vm, absl::string_view id, absl::string_view initial_configuration,
+           Upstream::ClusterManager& cluster_manager, Event::Dispatcher& dispatcher)
+    : cluster_manager_(cluster_manager), dispatcher_(dispatcher),
+      initial_configuration_(initial_configuration) {
   wasm_vm_ = Common::Wasm::createWasmVm(vm);
   id_ = std::string(id);
   if (wasm_vm_) {
@@ -1018,7 +1019,8 @@ void Wasm::getFunctions() {
 #undef _GET
 }
 
-Wasm::Wasm(const Wasm& wasm) {
+Wasm::Wasm(const Wasm& wasm)
+    : cluster_manager_(wasm.cluster_manager_), dispatcher_(wasm.dispatcher_) {
   wasm_vm_ = wasm.wasmVm()->clone();
   general_context_ = createContext();
   getFunctions();
@@ -1030,6 +1032,8 @@ bool Wasm::initialize(const std::string& code, absl::string_view name, bool allo
   auto ok = wasm_vm_->initialize(code, name, allow_precompiled);
   if (!ok)
     return false;
+  code_ = code;
+  allow_precompiled_ = allow_precompiled;
   getFunctions();
   return true;
 }
@@ -1046,8 +1050,8 @@ void Wasm::start() { general_context_->onStart(); }
 void Wasm::setTickPeriod(std::chrono::milliseconds tick_period) {
   bool was_running = timer_ && tick_period_.count() > 0;
   tick_period_ = tick_period;
-  if (dispatcher_ && tick_ && tick_period_.count() > 0 && !was_running) {
-    timer_ = dispatcher_->createTimer([this]() { this->tickHandler(); });
+  if (tick_ && tick_period_.count() > 0 && !was_running) {
+    timer_ = dispatcher_.createTimer([this]() { this->tickHandler(); });
     timer_->enableTimer(tick_period_);
   }
 }
@@ -1213,10 +1217,12 @@ std::unique_ptr<WasmVm> createWasmVm(absl::string_view wasm_vm) {
   }
 }
 
-std::unique_ptr<Wasm> createWasm(absl::string_view id,
-                                 const envoy::config::wasm::v2::VmConfig& vm_config,
-                                 Api::Api& api) {
-  auto wasm = std::make_unique<Wasm>(vm_config.vm(), id);
+static Wasm* createWasmInternal(absl::string_view id,
+                                const envoy::config::wasm::v2::VmConfig& vm_config,
+                                Upstream::ClusterManager& cluster_manager,
+                                Event::Dispatcher& dispatcher, Api::Api& api) {
+  auto wasm =
+      new Wasm(vm_config.vm(), id, vm_config.initial_configuration(), cluster_manager, dispatcher);
   const auto& code = Config::DataSource::read(vm_config.code(), true, api);
   const auto& path = Config::DataSource::getPath(vm_config.code())
                          .value_or(code.empty() ? EMPTY_STRING : INLINE_STRING);
@@ -1230,16 +1236,33 @@ std::unique_ptr<Wasm> createWasm(absl::string_view id,
   return wasm;
 }
 
-std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm,
-                                            const envoy::config::wasm::v2::VmConfig& vm_config,
-                                            Event::Dispatcher& dispatcher,
-                                            absl::string_view configuration, Api::Api& api) {
+std::shared_ptr<Wasm> createWasm(absl::string_view id,
+                                 const envoy::config::wasm::v2::VmConfig& vm_config,
+                                 Upstream::ClusterManager& cluster_manager,
+                                 Event::Dispatcher& dispatcher, Api::Api& api) {
+  return std::shared_ptr<Wasm>(createWasmInternal(id, vm_config, cluster_manager, dispatcher, api));
+}
+
+std::unique_ptr<Wasm> createWasmUnique(absl::string_view id,
+                                       const envoy::config::wasm::v2::VmConfig& vm_config,
+                                       Upstream::ClusterManager& cluster_manager,
+                                       Event::Dispatcher& dispatcher, Api::Api& api) {
+  return std::unique_ptr<Wasm>(createWasmInternal(id, vm_config, cluster_manager, dispatcher, api));
+}
+
+std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm, absl::string_view configuration,
+                                            Event::Dispatcher& dispatcher) {
   std::shared_ptr<Wasm> wasm;
   if (base_wasm.wasmVm()->clonable()) {
     wasm = std::make_shared<Wasm>(base_wasm);
   } else {
-    wasm = createWasm(base_wasm.id(), vm_config, api);
-    wasm->setDispatcher(dispatcher);
+    wasm = std::make_shared<Wasm>(base_wasm.wasmVm()->vm(), base_wasm.id(),
+                                  base_wasm.initial_configuration(), base_wasm.clusterManager(),
+                                  dispatcher);
+    if (!wasm->initialize(base_wasm.code(), base_wasm.id(), base_wasm.allow_precompiled())) {
+      throw WasmException("Failed to initialize WASM code");
+    }
+    wasm->configure(base_wasm.initial_configuration());
   }
   wasm->configure(configuration);
   if (!wasm->id().empty())
diff --git a/source/extensions/common/wasm/wasm.h b/source/extensions/common/wasm/wasm.h
index 0bede1a398f32..ec65c5772274c 100644
--- a/source/extensions/common/wasm/wasm.h
+++ b/source/extensions/common/wasm/wasm.h
@@ -53,7 +53,7 @@ class Context : public Http::StreamFilter,
 
   Wasm* wasm() const { return wasm_; }
   WasmVm* wasmVm() const;
-  Upstream::ClusterManager* clusterManager() const;
+  Upstream::ClusterManager& clusterManager() const;
   uint32_t id() const { return id_; }
   const StreamInfo::StreamInfo& streamInfo() const;
 
@@ -252,15 +252,11 @@ class Wasm : public Envoy::Server::Wasm,
              public Logger::Loggable<Logger::Id::wasm>,
              public std::enable_shared_from_this<Wasm> {
 public:
-  Wasm(absl::string_view vm, absl::string_view id);
+  Wasm(absl::string_view vm, absl::string_view id, absl::string_view initial_configuration,
+       Upstream::ClusterManager& cluster_manager, Event::Dispatcher& dispatcher);
   Wasm(const Wasm& other);
   ~Wasm() {}
 
-  void setDispatcher(Event::Dispatcher& dispatcher) { dispatcher_ = &dispatcher; }
-  void setClusterManager(Upstream::ClusterManager& clusterManager) {
-    clusterManager_ = &clusterManager;
-  }
-
   bool initialize(const std::string& code, absl::string_view name, bool allow_precompiled);
   void configure(absl::string_view configuration);
   void start();
@@ -271,7 +267,7 @@ class Wasm : public Envoy::Server::Wasm,
   absl::string_view id() const { return id_; }
   WasmVm* wasmVm() const { return wasm_vm_.get(); }
   Context* generalContext() const { return general_context_.get(); }
-  Upstream::ClusterManager* clusterManager() const { return clusterManager_; }
+  Upstream::ClusterManager& clusterManager() const { return cluster_manager_; }
 
   std::shared_ptr<Context> createContext() { return std::make_shared<Context>(this); }
 
@@ -280,6 +276,13 @@ class Wasm : public Envoy::Server::Wasm,
 
   uint32_t allocContextId();
 
+  const std::string& code() const { return code_; }
+  const std::string& initial_configuration() const { return initial_configuration_; }
+  bool allow_precompiled() const { return allow_precompiled_; }
+  void setInitialConfiguration(const std::string& initial_configuration) {
+    initial_configuration_ = initial_configuration;
+  }
+
   //
   // AccessLog::Instance
   //
@@ -296,8 +299,8 @@ class Wasm : public Envoy::Server::Wasm,
 
   void getFunctions();
 
-  Event::Dispatcher* dispatcher_ = nullptr;
-  Upstream::ClusterManager* clusterManager_ = nullptr;
+  Upstream::ClusterManager& cluster_manager_;
+  Event::Dispatcher& dispatcher_;
   std::string id_;
   std::string context_id_filter_state_data_name_;
   uint32_t next_context_id_ = 0;
@@ -330,10 +333,15 @@ class Wasm : public Envoy::Server::Wasm,
   WasmContextCall0Void onDone_;
   WasmContextCall0Void onLog_;
   WasmContextCall0Void onDelete_;
+
+  // Used by the base_wasm to enable non-clonable thread local Wasm(s) to be constructed.
+  std::string code_;
+  std::string initial_configuration_;
+  bool allow_precompiled_ = false;
 };
 
 inline WasmVm* Context::wasmVm() const { return wasm_->wasmVm(); }
-inline Upstream::ClusterManager* Context::clusterManager() const { return wasm_->clusterManager(); }
+inline Upstream::ClusterManager& Context::clusterManager() const { return wasm_->clusterManager(); }
 
 inline const ProtobufWkt::Struct& getMetadata(Http::StreamFilterCallbacks* callbacks) {
   if (callbacks->route() == nullptr || callbacks->route()->routeEntry() == nullptr) {
@@ -445,13 +453,18 @@ std::unique_ptr<WasmVm> createWasmVm(absl::string_view vm);
 
 // Create a new Wasm VM not attached to any thread. Note: 'id' may be empty if this VM will not be
 // shared by APIs (e.g. HTTP Filter + AccessLog).
-std::unique_ptr<Wasm> createWasm(absl::string_view id,
-                                 const envoy::config::wasm::v2::VmConfig& vm_config, Api::Api& api);
+std::shared_ptr<Wasm> createWasm(absl::string_view id,
+                                 const envoy::config::wasm::v2::VmConfig& vm_config,
+                                 Upstream::ClusterManager& cluster_manager,
+                                 Event::Dispatcher& dispatcher, Api::Api& api);
+// This is required for the singleton service Wasm instance.
+std::unique_ptr<Wasm> createWasmUnique(absl::string_view id,
+                                       const envoy::config::wasm::v2::VmConfig& vm_config,
+                                       Upstream::ClusterManager& cluster_manager,
+                                       Event::Dispatcher& dispatcher, Api::Api& api);
 // Create a ThreadLocal VM from an existing VM (e.g. from createWasm() above).
-std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm,
-                                            const envoy::config::wasm::v2::VmConfig& vm_config,
-                                            Event::Dispatcher& dispatcher,
-                                            absl::string_view configuration, Api::Api& api);
+std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm, absl::string_view configuration,
+                                            Event::Dispatcher& dispatcher);
 // Get an existing ThreadLocal VM matching 'id'.
 std::shared_ptr<Wasm> getThreadLocalWasm(absl::string_view id, absl::string_view configuration);
 
diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc
index ae4626128fe1a..1c68c65edecac 100644
--- a/source/extensions/filters/http/wasm/wasm_filter.cc
+++ b/source/extensions/filters/http/wasm/wasm_filter.cc
@@ -14,26 +14,26 @@ namespace Extensions {
 namespace HttpFilters {
 namespace Wasm {
 
-FilterConfig::FilterConfig(const envoy::config::filter::http::wasm::v2::Wasm& proto_config,
+FilterConfig::FilterConfig(const envoy::config::filter::http::wasm::v2::Wasm& config,
                            Server::Configuration::FactoryContext& context)
     : tls_slot_(context.threadLocal().allocateSlot()) {
-  if (proto_config.has_vm_config()) {
-    auto base_wasm = std::shared_ptr<Common::Wasm::Wasm>(
-        Common::Wasm::createWasm(proto_config.id(), proto_config.vm_config(), context.api())
-            .release());
-    tls_slot_->set([&proto_config, base_wasm, &context](Event::Dispatcher& dispatcher) {
-      auto result = Extensions::Common::Wasm::createThreadLocalWasm(
-          *base_wasm, proto_config.vm_config(), dispatcher, proto_config.configuration(),
-          context.api());
-      result->setClusterManager(context.clusterManager());
+  auto id = config.id();
+  auto configuration = std::make_shared<std::string>(config.configuration());
+  if (config.has_vm_config()) {
+    auto base_wasm = Common::Wasm::createWasm(id, config.vm_config(), context.clusterManager(),
+                                              context.dispatcher(), context.api());
+    // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.
+    tls_slot_->set([base_wasm, configuration](Event::Dispatcher& dispatcher) {
+      auto result =
+          Extensions::Common::Wasm::createThreadLocalWasm(*base_wasm, *configuration, dispatcher);
       return std::static_pointer_cast<ThreadLocal::ThreadLocalObject>(result);
     });
   } else {
-    tls_slot_->set([&proto_config, &context](Event::Dispatcher& dispatcher) {
-      auto result = Extensions::Common::Wasm::getThreadLocalWasm(proto_config.id(),
-                                                                 proto_config.configuration());
-      result->setDispatcher(dispatcher);
-      result->setClusterManager(context.clusterManager());
+    if (id.empty()) {
+      throw Common::Wasm::WasmVmException("No WASM VM Id or vm_config specified");
+    }
+    tls_slot_->set([id, configuration](Event::Dispatcher&) {
+      auto result = Extensions::Common::Wasm::getThreadLocalWasm(id, *configuration);
       return std::static_pointer_cast<ThreadLocal::ThreadLocalObject>(result);
     });
   }
diff --git a/source/extensions/wasm/config.cc b/source/extensions/wasm/config.cc
index 4e7045f32c6d6..428c5ad9a0223 100644
--- a/source/extensions/wasm/config.cc
+++ b/source/extensions/wasm/config.cc
@@ -18,19 +18,23 @@ static const std::string INLINE_STRING = "<inline>";
 
 Server::WasmPtr WasmFactory::createWasm(const envoy::config::wasm::v2::WasmConfig& config,
                                         Server::Configuration::WasmFactoryContext& context) {
+  auto wasm =
+      Common::Wasm::createWasmUnique(config.id(), config.vm_config(), context.clusterManager(),
+                                     context.dispatcher(), context.api());
   if (config.singleton()) {
-    auto wasm = Common::Wasm::createWasm(config.id(), config.vm_config(), context.api());
-    wasm->setDispatcher(context.dispatcher());
-    return Server::WasmPtr(wasm.release());
+    // Return the WASM VM which will be stored as a singleton by the Server.
+    return wasm;
   }
+  auto configuration = std::make_shared<std::string>(config.configuration());
+  auto base_wasm = std::shared_ptr<Common::Wasm::Wasm>(wasm.release());
   // Per-thread WASM VM.
-  auto base_wasm = Common::Wasm::createWasm(config.id(), config.vm_config(), context.api());
+  // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.
+  // NB: no need to keep the resulting slot as Wasm is cached on each thread.
   context.threadLocal().allocateSlot()->set(
-      [&config, &context, &base_wasm](Event::Dispatcher& dispatcher) {
-        // NB: no need to store the result as it is cached on each thread.
-        Extensions::Common::Wasm::createThreadLocalWasm(*base_wasm, config.vm_config(), dispatcher,
-                                                        config.configuration(), context.api());
-        return nullptr;
+      [base_wasm, configuration](Event::Dispatcher& dispatcher) {
+        auto wasm =
+            Extensions::Common::Wasm::createThreadLocalWasm(*base_wasm, *configuration, dispatcher);
+        return wasm;
       });
   // Do not return this WASM VM since this is per-thread. Returning it would indicate that this is a
   // singleton.
diff --git a/source/server/server.cc b/source/server/server.cc
index 08b6e38379a76..c878f6a95d113 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -320,8 +320,8 @@ void InstanceImpl::initialize(Options& options,
   if (bootstrap_.wasm_service_size() > 0) {
     auto factory = Registry::FactoryRegistry<Configuration::WasmFactory>::getFactory("envoy.wasm");
     if (factory) {
-      Configuration::WasmFactoryContextImpl wasm_factory_context(*dispatcher_, thread_local_,
-                                                                 api());
+      Configuration::WasmFactoryContextImpl wasm_factory_context(clusterManager(), *dispatcher_,
+                                                                 thread_local_, api());
       for (auto& config : bootstrap_.wasm_service()) {
         auto wasm = factory->createWasm(config, wasm_factory_context);
         if (wasm) {
diff --git a/source/server/wasm_config_impl.h b/source/server/wasm_config_impl.h
index 78726fe005be0..a9f7f7ffdd1a2 100644
--- a/source/server/wasm_config_impl.h
+++ b/source/server/wasm_config_impl.h
@@ -9,15 +9,17 @@ namespace Configuration {
 
 class WasmFactoryContextImpl : public WasmFactoryContext {
 public:
-  WasmFactoryContextImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls,
-                         Api::Api& api)
-      : dispatcher_(dispatcher), tls_(tls), api_(api) {}
+  WasmFactoryContextImpl(Upstream::ClusterManager& cluster_manager, Event::Dispatcher& dispatcher,
+                         ThreadLocal::SlotAllocator& tls, Api::Api& api)
+      : cluster_manager_(cluster_manager), dispatcher_(dispatcher), tls_(tls), api_(api) {}
 
+  Upstream::ClusterManager& clusterManager() override { return cluster_manager_; }
   Event::Dispatcher& dispatcher() override { return dispatcher_; }
   ThreadLocal::SlotAllocator& threadLocal() override { return tls_; }
   Api::Api& api() override { return api_; }
 
 private:
+  Upstream::ClusterManager& cluster_manager_;
   Event::Dispatcher& dispatcher_;
   ThreadLocal::SlotAllocator& tls_;
   Api::Api& api_;

From 90f95d654c05c43802dcc7b5ce99a62aaf2cb0bf Mon Sep 17 00:00:00 2001
From: John Plevyak <jplevyak@gmail.com>
Date: Mon, 11 Feb 2019 14:44:48 -0800
Subject: [PATCH 2/6] Fix WASM tests for the new clonable API.

---
 source/extensions/common/wasm/wasm.cc         | 14 ++++-----
 .../filters/http/wasm/wasm_filter_test.cc     |  5 ++--
 test/extensions/wasm/BUILD                    |  2 ++
 test/extensions/wasm/config_test.cc           | 29 ++++++++++++-------
 test/extensions/wasm/wasm_test.cc             | 11 +++++--
 5 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc
index 5860bac642a0b..69e5e10479221 100644
--- a/source/extensions/common/wasm/wasm.cc
+++ b/source/extensions/common/wasm/wasm.cc
@@ -1217,12 +1217,12 @@ std::unique_ptr<WasmVm> createWasmVm(absl::string_view wasm_vm) {
   }
 }
 
-static Wasm* createWasmInternal(absl::string_view id,
-                                const envoy::config::wasm::v2::VmConfig& vm_config,
-                                Upstream::ClusterManager& cluster_manager,
-                                Event::Dispatcher& dispatcher, Api::Api& api) {
-  auto wasm =
-      new Wasm(vm_config.vm(), id, vm_config.initial_configuration(), cluster_manager, dispatcher);
+static std::unique_ptr<Wasm> createWasmInternal(absl::string_view id,
+                                                const envoy::config::wasm::v2::VmConfig& vm_config,
+                                                Upstream::ClusterManager& cluster_manager,
+                                                Event::Dispatcher& dispatcher, Api::Api& api) {
+  auto wasm = std::make_unique<Wasm>(vm_config.vm(), id, vm_config.initial_configuration(),
+                                     cluster_manager, dispatcher);
   const auto& code = Config::DataSource::read(vm_config.code(), true, api);
   const auto& path = Config::DataSource::getPath(vm_config.code())
                          .value_or(code.empty() ? EMPTY_STRING : INLINE_STRING);
@@ -1247,7 +1247,7 @@ std::unique_ptr<Wasm> createWasmUnique(absl::string_view id,
                                        const envoy::config::wasm::v2::VmConfig& vm_config,
                                        Upstream::ClusterManager& cluster_manager,
                                        Event::Dispatcher& dispatcher, Api::Api& api) {
-  return std::unique_ptr<Wasm>(createWasmInternal(id, vm_config, cluster_manager, dispatcher, api));
+  return createWasmInternal(id, vm_config, cluster_manager, dispatcher, api);
 }
 
 std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm, absl::string_view configuration,
diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc
index 0a0e2f34dc955..cbec13b4347a7 100644
--- a/test/extensions/filters/http/wasm/wasm_filter_test.cc
+++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc
@@ -53,13 +53,12 @@ class WasmHttpFilterTest : public testing::Test {
     proto_config.mutable_vm_config()->mutable_code()->set_inline_bytes(code);
     Stats::IsolatedStoreImpl stats_store;
     Api::ApiPtr api = Api::createApiForTest(stats_store);
-    wasm_ = Extensions::Common::Wasm::createWasm(proto_config.id(), proto_config.vm_config(), *api);
-    wasm_->setClusterManager(cluster_manager_);
+    wasm_ = Extensions::Common::Wasm::createWasmUnique(proto_config.id(), proto_config.vm_config(),
+                                                       cluster_manager_, dispatcher_, *api);
   }
 
   void setupFilter() {
     wasm_->setGeneralContext(std::make_unique<TestFilter>(wasm_.get()));
-    ;
     filter_ = std::make_unique<TestFilter>(wasm_.get());
   }
 
diff --git a/test/extensions/wasm/BUILD b/test/extensions/wasm/BUILD
index f06b92436a0ae..d4897d99376e8 100644
--- a/test/extensions/wasm/BUILD
+++ b/test/extensions/wasm/BUILD
@@ -25,6 +25,7 @@ envoy_extension_cc_test(
         "//source/common/stats:stats_lib",
         "//source/extensions/common/wasm:wasm_lib",
         "//source/extensions/wasm:config",
+        "//test/mocks/upstream:upstream_mocks",
         "//test/test_common:environment_lib",
         "//test/test_common:simulated_time_system_lib",
     ],
@@ -45,6 +46,7 @@ envoy_extension_cc_test(
         "//source/server:wasm_config_lib",
         "//test/mocks/event:event_mocks",
         "//test/mocks/thread_local:thread_local_mocks",
+        "//test/mocks/upstream:upstream_mocks",
         "//test/test_common:environment_lib",
         "@envoy_api//envoy/config/wasm/v2:wasm_cc",
     ],
diff --git a/test/extensions/wasm/config_test.cc b/test/extensions/wasm/config_test.cc
index 46c370b506dd5..f229f49c3f9ad 100644
--- a/test/extensions/wasm/config_test.cc
+++ b/test/extensions/wasm/config_test.cc
@@ -11,6 +11,7 @@
 
 #include "test/mocks/event/mocks.h"
 #include "test/mocks/thread_local/mocks.h"
+#include "test/mocks/upstream/mocks.h"
 #include "test/test_common/environment.h"
 
 #include "gtest/gtest.h"
@@ -28,15 +29,15 @@ TEST(WasmFactoryTest, CreateWasmFromWASM) {
   config.mutable_vm_config()->mutable_code()->set_filename(
       TestEnvironment::substitute("{{ test_rundir }}/test/extensions/wasm/test_data/logging.wasm"));
   config.set_singleton(true);
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   ThreadLocal::MockInstance tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
   auto wasm = factory->createWasm(config, context);
   EXPECT_NE(wasm, nullptr);
 }
-
 TEST(WasmFactoryTest, CreateWasmFromPrecompiledWASM) {
   auto factory =
       Registry::FactoryRegistry<Server::Configuration::WasmFactory>::getFactory("envoy.wasm");
@@ -47,11 +48,12 @@ TEST(WasmFactoryTest, CreateWasmFromPrecompiledWASM) {
       TestEnvironment::substitute("{{ test_rundir }}/test/extensions/wasm/test_data/logging.wasm"));
   config.mutable_vm_config()->set_allow_precompiled(true);
   config.set_singleton(true);
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   ThreadLocal::MockInstance tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
   auto wasm = factory->createWasm(config, context);
   EXPECT_NE(wasm, nullptr);
 }
@@ -65,11 +67,12 @@ TEST(WasmFactoryTest, CreateWasmFromWASMPerThread) {
   config.mutable_vm_config()->mutable_code()->set_filename(
       TestEnvironment::substitute("{{ test_rundir }}/test/extensions/wasm/test_data/logging.wasm"));
   config.set_id("test_id");
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   testing::NiceMock<ThreadLocal::MockInstance> tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
   auto wasm = factory->createWasm(config, context);
   EXPECT_EQ(wasm, nullptr);
 }
@@ -83,11 +86,12 @@ TEST(WasmFactoryTest, CreateWasmFromWAT) {
   config.mutable_vm_config()->mutable_code()->set_filename(
       TestEnvironment::substitute("{{ test_rundir }}/test/extensions/wasm/test_data/logging.wat"));
   config.set_singleton(true);
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   ThreadLocal::MockInstance tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
   auto wasm = factory->createWasm(config, context);
   EXPECT_NE(wasm, nullptr);
 }
@@ -117,11 +121,12 @@ TEST(WasmFactoryTest, CreateWasmFromInlineWAT) {
       " )");
   config.set_singleton(true);
 
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   ThreadLocal::MockInstance tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
   auto wasm = factory->createWasm(config, context);
   EXPECT_NE(wasm, nullptr);
 }
@@ -151,11 +156,12 @@ TEST(WasmFactoryTest, CreateWasmFromInlineWATWithAlias) {
       " )");
   config.set_singleton(true);
 
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   ThreadLocal::MockInstance tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
   auto wasm = factory->createWasm(config, context);
   EXPECT_NE(wasm, nullptr);
 }
@@ -185,11 +191,12 @@ TEST(WasmFactoryTest, CreateWasmFromInlineWATWithUnderscoreAlias) {
       " )");
   config.set_singleton(true);
 
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   ThreadLocal::MockInstance tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
   auto wasm = factory->createWasm(config, context);
   EXPECT_NE(wasm, nullptr);
 }
@@ -219,13 +226,15 @@ TEST(WasmFactoryTest, MissingImport) {
       " )");
   config.set_singleton(true);
 
+  Upstream::MockClusterManager cluster_manager;
   Event::MockDispatcher dispatcher;
   ThreadLocal::MockInstance tls;
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
-  Server::Configuration::WasmFactoryContextImpl context(dispatcher, tls, *api);
+  Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
+  Server::WasmPtr wasm;
   EXPECT_THROW_WITH_MESSAGE(
-      factory->createWasm(config, context), Extensions::Common::Wasm::WasmException,
+      wasm = factory->createWasm(config, context), Extensions::Common::Wasm::WasmException,
       "Failed to load WASM module due to a missing import: env.missing func (i32, i32, i32)->()");
 }
 
diff --git a/test/extensions/wasm/wasm_test.cc b/test/extensions/wasm/wasm_test.cc
index fddfbdb6eba27..2de60a5a84c5b 100644
--- a/test/extensions/wasm/wasm_test.cc
+++ b/test/extensions/wasm/wasm_test.cc
@@ -5,6 +5,7 @@
 
 #include "extensions/common/wasm/wasm.h"
 
+#include "test/mocks/upstream/mocks.h"
 #include "test/test_common/environment.h"
 #include "test/test_common/simulated_time_system.h"
 #include "test/test_common/utility.h"
@@ -31,8 +32,10 @@ TEST(WasmTest, Logging) {
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
   Event::SimulatedTimeSystem time_system;
+  Upstream::MockClusterManager cluster_manager;
   Event::DispatcherImpl dispatcher(time_system, *api);
-  auto wasm = std::make_unique<Extensions::Common::Wasm::Wasm>("envoy.wasm.vm.wavm", "");
+  auto wasm = std::make_unique<Extensions::Common::Wasm::Wasm>("envoy.wasm.vm.wavm", "", "",
+                                                               cluster_manager, dispatcher);
   EXPECT_NE(wasm, nullptr);
   const auto code = TestEnvironment::readFileToStringForTest(
       TestEnvironment::substitute("{{ test_rundir }}/test/extensions/wasm/test_data/logging.wasm"));
@@ -52,12 +55,14 @@ TEST(WasmTest, Logging) {
   wasm->tickHandler();
 }
 
-TEST(WasmTest, BadkSignature) {
+TEST(WasmTest, BadSignature) {
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
   Event::SimulatedTimeSystem time_system;
+  Upstream::MockClusterManager cluster_manager;
   Event::DispatcherImpl dispatcher(time_system, *api);
-  auto wasm = std::make_unique<Extensions::Common::Wasm::Wasm>("envoy.wasm.vm.wavm", "");
+  auto wasm = std::make_unique<Extensions::Common::Wasm::Wasm>("envoy.wasm.vm.wavm", "", "",
+                                                               cluster_manager, dispatcher);
   EXPECT_NE(wasm, nullptr);
   const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(
       "{{ test_rundir }}/test/extensions/wasm/test_data/bad_signature.wasm"));

From 9be6c4c2aa82bc46a2c7459deb9a40d70d28cae0 Mon Sep 17 00:00:00 2001
From: John Plevyak <jplevyak@gmail.com>
Date: Mon, 11 Feb 2019 16:05:02 -0800
Subject: [PATCH 3/6] Use weak pointer to protect timer callback for WASM.

---
 source/extensions/common/wasm/wasm.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc
index 69e5e10479221..51b78d6a07b6a 100644
--- a/source/extensions/common/wasm/wasm.cc
+++ b/source/extensions/common/wasm/wasm.cc
@@ -1051,7 +1051,11 @@ void Wasm::setTickPeriod(std::chrono::milliseconds tick_period) {
   bool was_running = timer_ && tick_period_.count() > 0;
   tick_period_ = tick_period;
   if (tick_ && tick_period_.count() > 0 && !was_running) {
-    timer_ = dispatcher_.createTimer([this]() { this->tickHandler(); });
+    timer_ = dispatcher_.createTimer([weak = std::weak_ptr<Wasm>(shared_from_this())]() {
+      auto shared = weak.lock();
+      if (shared)
+        shared->tickHandler();
+    });
     timer_->enableTimer(tick_period_);
   }
 }

From 4daff471482c7e729dc2f97bd85fd5837d55d485 Mon Sep 17 00:00:00 2001
From: John Plevyak <jplevyak@gmail.com>
Date: Mon, 11 Feb 2019 19:14:58 -0800
Subject: [PATCH 4/6] Use shared_ptr for all Common::Wasm::Wasm pointers for
 consistency and safety with std::enable_shared_from_this().

---
 include/envoy/server/wasm.h                   |  2 +-
 include/envoy/server/wasm_config.h            |  6 ++---
 source/extensions/common/wasm/wasm.cc         | 24 ++++---------------
 source/extensions/common/wasm/wasm.h          | 13 ++++------
 source/extensions/wasm/config.cc              | 23 ++++++++----------
 source/extensions/wasm/config.h               |  4 ++--
 source/server/server.h                        |  2 +-
 .../filters/http/wasm/wasm_filter_test.cc     |  6 ++---
 test/extensions/wasm/config_test.cc           |  2 +-
 9 files changed, 31 insertions(+), 51 deletions(-)

diff --git a/include/envoy/server/wasm.h b/include/envoy/server/wasm.h
index 74f0dd9af7421..feb8aad78dc19 100644
--- a/include/envoy/server/wasm.h
+++ b/include/envoy/server/wasm.h
@@ -16,7 +16,7 @@ class Wasm {
   virtual ~Wasm() {}
 };
 
-typedef std::unique_ptr<Wasm> WasmPtr;
+typedef std::shared_ptr<Wasm> WasmSharedPtr;
 
 } // namespace Server
 } // namespace Envoy
diff --git a/include/envoy/server/wasm_config.h b/include/envoy/server/wasm_config.h
index c8520c4954bec..f32140331a5ba 100644
--- a/include/envoy/server/wasm_config.h
+++ b/include/envoy/server/wasm_config.h
@@ -54,12 +54,12 @@ class WasmFactory {
    * @param config const ProtoBuf::Message& supplies the config for the resource monitor
    *        implementation.
    * @param context WasmFactoryContext& supplies the resource monitor's context.
-   * @return WasmPtr a singleton Wasm servive. May be be nullptr if per silo.
+   * @return WasmSharedPtr a singleton Wasm servive. May be be nullptr if per silo.
    * @throw EnvoyException if the implementation is unable to produce an instance with
    *        the provided parameters.
    */
-  virtual WasmPtr createWasm(const envoy::config::wasm::v2::WasmConfig& config,
-                             WasmFactoryContext& context) PURE;
+  virtual WasmSharedPtr createWasm(const envoy::config::wasm::v2::WasmConfig& config,
+                                   WasmFactoryContext& context) PURE;
 };
 
 } // namespace Configuration
diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc
index 51b78d6a07b6a..799f55fc9d78c 100644
--- a/source/extensions/common/wasm/wasm.cc
+++ b/source/extensions/common/wasm/wasm.cc
@@ -1221,11 +1221,11 @@ std::unique_ptr<WasmVm> createWasmVm(absl::string_view wasm_vm) {
   }
 }
 
-static std::unique_ptr<Wasm> createWasmInternal(absl::string_view id,
-                                                const envoy::config::wasm::v2::VmConfig& vm_config,
-                                                Upstream::ClusterManager& cluster_manager,
-                                                Event::Dispatcher& dispatcher, Api::Api& api) {
-  auto wasm = std::make_unique<Wasm>(vm_config.vm(), id, vm_config.initial_configuration(),
+std::shared_ptr<Wasm> createWasm(absl::string_view id,
+                                 const envoy::config::wasm::v2::VmConfig& vm_config,
+                                 Upstream::ClusterManager& cluster_manager,
+                                 Event::Dispatcher& dispatcher, Api::Api& api) {
+  auto wasm = std::make_shared<Wasm>(vm_config.vm(), id, vm_config.initial_configuration(),
                                      cluster_manager, dispatcher);
   const auto& code = Config::DataSource::read(vm_config.code(), true, api);
   const auto& path = Config::DataSource::getPath(vm_config.code())
@@ -1240,20 +1240,6 @@ static std::unique_ptr<Wasm> createWasmInternal(absl::string_view id,
   return wasm;
 }
 
-std::shared_ptr<Wasm> createWasm(absl::string_view id,
-                                 const envoy::config::wasm::v2::VmConfig& vm_config,
-                                 Upstream::ClusterManager& cluster_manager,
-                                 Event::Dispatcher& dispatcher, Api::Api& api) {
-  return std::shared_ptr<Wasm>(createWasmInternal(id, vm_config, cluster_manager, dispatcher, api));
-}
-
-std::unique_ptr<Wasm> createWasmUnique(absl::string_view id,
-                                       const envoy::config::wasm::v2::VmConfig& vm_config,
-                                       Upstream::ClusterManager& cluster_manager,
-                                       Event::Dispatcher& dispatcher, Api::Api& api) {
-  return createWasmInternal(id, vm_config, cluster_manager, dispatcher, api);
-}
-
 std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm, absl::string_view configuration,
                                             Event::Dispatcher& dispatcher) {
   std::shared_ptr<Wasm> wasm;
diff --git a/source/extensions/common/wasm/wasm.h b/source/extensions/common/wasm/wasm.h
index ec65c5772274c..06f61a6b107e6 100644
--- a/source/extensions/common/wasm/wasm.h
+++ b/source/extensions/common/wasm/wasm.h
@@ -448,23 +448,20 @@ class WasmVm : public Logger::Loggable<Logger::Id::wasm> {
   }
 };
 
-// Create a new WASM VM of the give type (e.g. "envoy.wasm.vm.wavm").
+// Create a new low-level WASM VM of the give type (e.g. "envoy.wasm.vm.wavm").
 std::unique_ptr<WasmVm> createWasmVm(absl::string_view vm);
 
-// Create a new Wasm VM not attached to any thread. Note: 'id' may be empty if this VM will not be
-// shared by APIs (e.g. HTTP Filter + AccessLog).
+// Create a high level Wasm VM with Envoy API support. Note: 'id' may be empty if this VM will not
+// be shared by APIs (e.g. HTTP Filter + AccessLog).
 std::shared_ptr<Wasm> createWasm(absl::string_view id,
                                  const envoy::config::wasm::v2::VmConfig& vm_config,
                                  Upstream::ClusterManager& cluster_manager,
                                  Event::Dispatcher& dispatcher, Api::Api& api);
-// This is required for the singleton service Wasm instance.
-std::unique_ptr<Wasm> createWasmUnique(absl::string_view id,
-                                       const envoy::config::wasm::v2::VmConfig& vm_config,
-                                       Upstream::ClusterManager& cluster_manager,
-                                       Event::Dispatcher& dispatcher, Api::Api& api);
+
 // Create a ThreadLocal VM from an existing VM (e.g. from createWasm() above).
 std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm, absl::string_view configuration,
                                             Event::Dispatcher& dispatcher);
+
 // Get an existing ThreadLocal VM matching 'id'.
 std::shared_ptr<Wasm> getThreadLocalWasm(absl::string_view id, absl::string_view configuration);
 
diff --git a/source/extensions/wasm/config.cc b/source/extensions/wasm/config.cc
index 428c5ad9a0223..099860d31c1a2 100644
--- a/source/extensions/wasm/config.cc
+++ b/source/extensions/wasm/config.cc
@@ -16,26 +16,23 @@ namespace Wasm {
 
 static const std::string INLINE_STRING = "<inline>";
 
-Server::WasmPtr WasmFactory::createWasm(const envoy::config::wasm::v2::WasmConfig& config,
-                                        Server::Configuration::WasmFactoryContext& context) {
-  auto wasm =
-      Common::Wasm::createWasmUnique(config.id(), config.vm_config(), context.clusterManager(),
-                                     context.dispatcher(), context.api());
+Server::WasmSharedPtr WasmFactory::createWasm(const envoy::config::wasm::v2::WasmConfig& config,
+                                              Server::Configuration::WasmFactoryContext& context) {
+  auto base_wasm =
+      Common::Wasm::createWasm(config.id(), config.vm_config(), context.clusterManager(),
+                               context.dispatcher(), context.api());
   if (config.singleton()) {
     // Return the WASM VM which will be stored as a singleton by the Server.
-    return wasm;
+    return base_wasm;
   }
   auto configuration = std::make_shared<std::string>(config.configuration());
-  auto base_wasm = std::shared_ptr<Common::Wasm::Wasm>(wasm.release());
   // Per-thread WASM VM.
   // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.
   // NB: no need to keep the resulting slot as Wasm is cached on each thread.
-  context.threadLocal().allocateSlot()->set(
-      [base_wasm, configuration](Event::Dispatcher& dispatcher) {
-        auto wasm =
-            Extensions::Common::Wasm::createThreadLocalWasm(*base_wasm, *configuration, dispatcher);
-        return wasm;
-      });
+  context.threadLocal().allocateSlot()->set([base_wasm,
+                                             configuration](Event::Dispatcher& dispatcher) {
+    return Extensions::Common::Wasm::createThreadLocalWasm(*base_wasm, *configuration, dispatcher);
+  });
   // Do not return this WASM VM since this is per-thread. Returning it would indicate that this is a
   // singleton.
   return nullptr;
diff --git a/source/extensions/wasm/config.h b/source/extensions/wasm/config.h
index fbcf9e35c166e..0fff42a46ce04 100644
--- a/source/extensions/wasm/config.h
+++ b/source/extensions/wasm/config.h
@@ -13,8 +13,8 @@ class WasmFactory : public Server::Configuration::WasmFactory {
 public:
   ~WasmFactory() override {}
   std::string name() override { return "envoy.wasm"; }
-  Server::WasmPtr createWasm(const envoy::config::wasm::v2::WasmConfig& config,
-                             Server::Configuration::WasmFactoryContext& context) override;
+  Server::WasmSharedPtr createWasm(const envoy::config::wasm::v2::WasmConfig& config,
+                                   Server::Configuration::WasmFactoryContext& context) override;
 };
 
 } // namespace Wasm
diff --git a/source/server/server.h b/source/server/server.h
index 7a4fdf121b38f..4b1fd7892fde5 100644
--- a/source/server/server.h
+++ b/source/server/server.h
@@ -240,7 +240,7 @@ class InstanceImpl : Logger::Loggable<Logger::Id::main>, public Instance {
   Upstream::ProdClusterInfoFactory info_factory_;
   Upstream::HdsDelegatePtr hds_delegate_;
   std::unique_ptr<OverloadManagerImpl> overload_manager_;
-  std::vector<std::unique_ptr<Wasm>> wasm_;
+  std::vector<std::shared_ptr<Wasm>> wasm_;
   std::unique_ptr<RunHelper> run_helper_;
   Envoy::MutexTracer* mutex_tracer_;
   Http::ContextImpl http_context_;
diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc
index cbec13b4347a7..e54b0462209a9 100644
--- a/test/extensions/filters/http/wasm/wasm_filter_test.cc
+++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc
@@ -53,8 +53,8 @@ class WasmHttpFilterTest : public testing::Test {
     proto_config.mutable_vm_config()->mutable_code()->set_inline_bytes(code);
     Stats::IsolatedStoreImpl stats_store;
     Api::ApiPtr api = Api::createApiForTest(stats_store);
-    wasm_ = Extensions::Common::Wasm::createWasmUnique(proto_config.id(), proto_config.vm_config(),
-                                                       cluster_manager_, dispatcher_, *api);
+    wasm_ = Extensions::Common::Wasm::createWasm(proto_config.id(), proto_config.vm_config(),
+                                                 cluster_manager_, dispatcher_, *api);
   }
 
   void setupFilter() {
@@ -65,7 +65,7 @@ class WasmHttpFilterTest : public testing::Test {
   NiceMock<ThreadLocal::MockInstance> tls_;
   NiceMock<Event::MockDispatcher> dispatcher_;
   Upstream::MockClusterManager cluster_manager_;
-  std::unique_ptr<Wasm> wasm_;
+  std::shared_ptr<Wasm> wasm_;
   std::unique_ptr<TestFilter> filter_;
   Http::MockStreamDecoderFilterCallbacks decoder_callbacks_;
   Http::MockStreamEncoderFilterCallbacks encoder_callbacks_;
diff --git a/test/extensions/wasm/config_test.cc b/test/extensions/wasm/config_test.cc
index f229f49c3f9ad..20f604a7247c0 100644
--- a/test/extensions/wasm/config_test.cc
+++ b/test/extensions/wasm/config_test.cc
@@ -232,7 +232,7 @@ TEST(WasmFactoryTest, MissingImport) {
   Stats::IsolatedStoreImpl stats_store;
   Api::ApiPtr api = Api::createApiForTest(stats_store);
   Server::Configuration::WasmFactoryContextImpl context(cluster_manager, dispatcher, tls, *api);
-  Server::WasmPtr wasm;
+  Server::WasmSharedPtr wasm;
   EXPECT_THROW_WITH_MESSAGE(
       wasm = factory->createWasm(config, context), Extensions::Common::Wasm::WasmException,
       "Failed to load WASM module due to a missing import: env.missing func (i32, i32, i32)->()");

From 852a96cfaf97a60ee374ee9957846e76ae6c8903 Mon Sep 17 00:00:00 2001
From: John Plevyak <jplevyak@gmail.com>
Date: Wed, 13 Feb 2019 13:24:55 -0800
Subject: [PATCH 5/6] Apply PR comment suggestions.

---
 include/envoy/server/wasm_config.h | 2 +-
 test/extensions/wasm/wasm_test.cc  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/envoy/server/wasm_config.h b/include/envoy/server/wasm_config.h
index f32140331a5ba..cd0e6c93f15d9 100644
--- a/include/envoy/server/wasm_config.h
+++ b/include/envoy/server/wasm_config.h
@@ -54,7 +54,7 @@ class WasmFactory {
    * @param config const ProtoBuf::Message& supplies the config for the resource monitor
    *        implementation.
    * @param context WasmFactoryContext& supplies the resource monitor's context.
-   * @return WasmSharedPtr a singleton Wasm servive. May be be nullptr if per silo.
+   * @return WasmSharedPtr a singleton Wasm service. May be be nullptr if per silo.
    * @throw EnvoyException if the implementation is unable to produce an instance with
    *        the provided parameters.
    */
diff --git a/test/extensions/wasm/wasm_test.cc b/test/extensions/wasm/wasm_test.cc
index 2de60a5a84c5b..dc386fabd363f 100644
--- a/test/extensions/wasm/wasm_test.cc
+++ b/test/extensions/wasm/wasm_test.cc
@@ -61,7 +61,7 @@ TEST(WasmTest, BadSignature) {
   Event::SimulatedTimeSystem time_system;
   Upstream::MockClusterManager cluster_manager;
   Event::DispatcherImpl dispatcher(time_system, *api);
-  auto wasm = std::make_unique<Extensions::Common::Wasm::Wasm>("envoy.wasm.vm.wavm", "", "",
+  auto wasm = std::make_shared<Extensions::Common::Wasm::Wasm>("envoy.wasm.vm.wavm", "", "",
                                                                cluster_manager, dispatcher);
   EXPECT_NE(wasm, nullptr);
   const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute(

From bb460e759dafb396c1b0e9c1d35503121b13db3d Mon Sep 17 00:00:00 2001
From: John Plevyak <jplevyak@gmail.com>
Date: Wed, 13 Feb 2019 13:28:36 -0800
Subject: [PATCH 6/6] Add comments for PR suggestions.

---
 source/extensions/access_loggers/wasm/config.cc    | 1 +
 source/extensions/filters/http/wasm/wasm_filter.cc | 1 +
 source/extensions/wasm/config.cc                   | 1 +
 3 files changed, 3 insertions(+)

diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc
index 13f640c8fc172..33e767374361e 100644
--- a/source/extensions/access_loggers/wasm/config.cc
+++ b/source/extensions/access_loggers/wasm/config.cc
@@ -30,6 +30,7 @@ WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_con
   auto configuration = std::make_shared<std::string>(config.configuration());
   auto tls_slot = context.threadLocal().allocateSlot();
   if (config.has_vm_config()) {
+    // Create a base WASM to verify that the code loads before setting/cloning the for the individual threads.
     auto base_wasm = Common::Wasm::createWasm(id, config.vm_config(), context.clusterManager(),
                                               context.dispatcher(), context.api());
     // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.
diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc
index 1c68c65edecac..46d97f8d8252b 100644
--- a/source/extensions/filters/http/wasm/wasm_filter.cc
+++ b/source/extensions/filters/http/wasm/wasm_filter.cc
@@ -20,6 +20,7 @@ FilterConfig::FilterConfig(const envoy::config::filter::http::wasm::v2::Wasm& co
   auto id = config.id();
   auto configuration = std::make_shared<std::string>(config.configuration());
   if (config.has_vm_config()) {
+    // Create a base WASM to verify that the code loads before setting/cloning the for the individual threads.
     auto base_wasm = Common::Wasm::createWasm(id, config.vm_config(), context.clusterManager(),
                                               context.dispatcher(), context.api());
     // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.
diff --git a/source/extensions/wasm/config.cc b/source/extensions/wasm/config.cc
index 099860d31c1a2..41eb29ce7b584 100644
--- a/source/extensions/wasm/config.cc
+++ b/source/extensions/wasm/config.cc
@@ -18,6 +18,7 @@ static const std::string INLINE_STRING = "<inline>";
 
 Server::WasmSharedPtr WasmFactory::createWasm(const envoy::config::wasm::v2::WasmConfig& config,
                                               Server::Configuration::WasmFactoryContext& context) {
+  // Create a base WASM to verify that the code loads before setting/cloning the for the individual threads.
   auto base_wasm =
       Common::Wasm::createWasm(config.id(), config.vm_config(), context.clusterManager(),
                                context.dispatcher(), context.api());