From 697dc19ce992d2f66309d5af12b4fe9438f8a410 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Sun, 25 Oct 2020 09:38:09 +0000 Subject: [PATCH 01/15] wasm: allow execution of multiple instances of the same plugin. Fixes #13690. Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 8 +-- .../extensions/access_loggers/wasm/config.cc | 4 +- .../wasm/wasm_access_log_impl.h | 20 ++++++-- source/extensions/bootstrap/wasm/config.cc | 2 +- source/extensions/bootstrap/wasm/config.h | 14 +++++- source/extensions/common/wasm/context.cc | 12 ++--- source/extensions/common/wasm/wasm.cc | 8 +-- source/extensions/common/wasm/wasm.h | 9 ++-- .../filters/http/wasm/wasm_filter.cc | 19 +++---- .../filters/http/wasm/wasm_filter.h | 3 +- .../filters/network/wasm/wasm_filter.cc | 14 ++++-- .../filters/network/wasm/wasm_filter.h | 6 ++- source/extensions/stat_sinks/wasm/config.cc | 4 +- .../stat_sinks/wasm/wasm_stat_sink_impl.h | 9 ++-- test/extensions/bootstrap/wasm/wasm_test.cc | 4 +- .../common/wasm/test_data/test_cpp.cc | 4 ++ test/extensions/common/wasm/wasm_test.cc | 15 +++--- .../filters/http/wasm/wasm_filter_test.cc | 50 +++++++++---------- .../filters/network/wasm/config_test.cc | 2 +- .../filters/network/wasm/wasm_filter_test.cc | 2 +- test/test_common/wasm_base.h | 8 +-- 21 files changed, 127 insertions(+), 90 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 677bfb4e9ecd..06ffb0737064 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -830,10 +830,10 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "d54b3795e7c3e61015dac2c2110b0da2be999b8e", - sha256 = "e95ad57b6b550b039d4baa35c896f7f523e427b7278ba56f22fac6e1bef8c7f0", + version = "fa18f0005afd47e515577af7fdfe94b8a0268860", + sha256 = "8dfb533513fbc5bb2493bae1b8dc825bcc2c31776c6b1761b1713ac1f133df5a", strip_prefix = "proxy-wasm-cpp-host-{version}", - urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], + urls = ["https://github.com/PiotrSikora/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], extensions = [ "envoy.access_loggers.wasm", @@ -842,7 +842,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - release_date = "2020-10-22", + release_date = "2020-10-29", cpe = "N/A", ), emscripten_toolchain = dict( diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc index 718adb0fad93..800b6484c93c 100644 --- a/source/extensions/access_loggers/wasm/config.cc +++ b/source/extensions/access_loggers/wasm/config.cc @@ -23,8 +23,6 @@ WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_con const auto& config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::wasm::v3::WasmAccessLog&>( proto_config, context.messageValidationVisitor()); - auto access_log = - std::make_shared(config.config().root_id(), nullptr, std::move(filter)); // Create a base WASM to verify that the code loads before setting/cloning the for the // individual threads. @@ -35,6 +33,8 @@ WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_con envoy::config::core::v3::TrafficDirection::UNSPECIFIED, context.localInfo(), nullptr /* listener_metadata */); + auto access_log = std::make_shared(plugin, nullptr, std::move(filter)); + auto callback = [access_log, &context, plugin](Common::Wasm::WasmHandleSharedPtr base_wasm) { auto tls_slot = context.threadLocal().allocateSlot(); diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index 5a7654b97bee..c0467fba790c 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -12,13 +12,15 @@ namespace Extensions { namespace AccessLoggers { namespace Wasm { +using Envoy::Extensions::Common::Wasm::PluginSharedPtr; using Envoy::Extensions::Common::Wasm::WasmHandle; class WasmAccessLog : public AccessLog::Instance { public: - WasmAccessLog(absl::string_view root_id, ThreadLocal::SlotPtr tls_slot, + WasmAccessLog(const PluginSharedPtr& plugin, ThreadLocal::SlotPtr tls_slot, AccessLog::FilterPtr filter) - : root_id_(root_id), tls_slot_(std::move(tls_slot)), filter_(std::move(filter)) {} + : plugin_(plugin), tls_slot_(std::move(tls_slot)), filter_(std::move(filter)) {} + void log(const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers, @@ -31,7 +33,7 @@ class WasmAccessLog : public AccessLog::Instance { } if (tls_slot_->get()) { - tls_slot_->getTyped().wasm()->log(root_id_, request_headers, response_headers, + tls_slot_->getTyped().wasm()->log(plugin_, request_headers, response_headers, response_trailers, stream_info); } } @@ -41,8 +43,18 @@ class WasmAccessLog : public AccessLog::Instance { tls_slot_ = std::move(tls_slot); } + ~WasmAccessLog() { + if (tls_slot_->get()) { + tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) + -> ThreadLocal::ThreadLocalObjectSharedPtr { + object->asType().wasm()->startShutdown(plugin); + return object; + }); + } + } + private: - std::string root_id_; + Common::Wasm::PluginSharedPtr plugin_; ThreadLocal::SlotPtr tls_slot_; AccessLog::FilterPtr filter_; }; diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 3cc0068b9a16..3d9b9fd1d799 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -48,7 +48,7 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con return std::static_pointer_cast( Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher)); }); - cb(std::make_unique(std::move(tls_slot))); + cb(std::make_unique(plugin, std::move(tls_slot))); }; if (!Common::Wasm::createWasm( diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index e70306746389..6cc4494e46da 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -19,9 +19,21 @@ namespace Wasm { class WasmService { public: WasmService(Common::Wasm::WasmHandleSharedPtr singleton) : singleton_(std::move(singleton)) {} - WasmService(ThreadLocal::SlotPtr tls_slot) : tls_slot_(std::move(tls_slot)) {} + WasmService(Common::Wasm::PluginSharedPtr plugin, ThreadLocal::SlotPtr tls_slot) + : plugin_(plugin), tls_slot_(std::move(tls_slot)) {} + + ~WasmService() { + if (tls_slot_ && tls_slot_->get()) { + tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) + -> ThreadLocal::ThreadLocalObjectSharedPtr { + object->asType().wasm()->startShutdown(plugin); + return object; + }); + } + } private: + Common::Wasm::PluginSharedPtr plugin_; Common::Wasm::WasmHandleSharedPtr singleton_; ThreadLocal::SlotPtr tls_slot_; }; diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index e6e4f8ae0f05..f5d0f3183a18 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -810,8 +810,8 @@ BufferInterface* Context::getBuffer(WasmBufferType type) { case WasmBufferType::VmConfiguration: return buffer_.set(wasm()->vm_configuration()); case WasmBufferType::PluginConfiguration: - if (plugin_) { - return buffer_.set(plugin_->plugin_configuration_); + if (temp_plugin_) { + return buffer_.set(temp_plugin_->plugin_configuration_); } return nullptr; case WasmBufferType::HttpRequestBody: @@ -1182,18 +1182,18 @@ bool Context::validateConfiguration(absl::string_view configuration, if (!wasm()->validate_configuration_) { return true; } - plugin_ = plugin_base; + temp_plugin_ = plugin_base; auto result = wasm() ->validate_configuration_(this, id_, static_cast(configuration.size())) .u64_ != 0; - plugin_.reset(); + temp_plugin_.reset(); return result; } absl::string_view Context::getConfiguration() { - if (plugin_) { - return plugin_->plugin_configuration_; + if (temp_plugin_) { + return temp_plugin_->plugin_configuration_; } else { return wasm()->vm_configuration(); } diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index ab2a45f0aaf7..7a2a2478e82a 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -243,16 +243,16 @@ ContextBase* Wasm::createRootContext(const std::shared_ptr& plugin) ContextBase* Wasm::createVmContext() { return new Context(this); } -void Wasm::log(absl::string_view root_id, const Http::RequestHeaderMap* request_headers, +void Wasm::log(const PluginSharedPtr& plugin, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers, const StreamInfo::StreamInfo& stream_info) { - auto context = getRootContext(root_id); + auto context = getRootContext(plugin, true); context->log(request_headers, response_headers, response_trailers, stream_info); } -void Wasm::onStatsUpdate(absl::string_view root_id, Envoy::Stats::MetricSnapshot& snapshot) { - auto context = getRootContext(root_id); +void Wasm::onStatsUpdate(const PluginSharedPtr& plugin, Envoy::Stats::MetricSnapshot& snapshot) { + auto context = getRootContext(plugin, true); context->onStatsUpdate(snapshot); } diff --git a/source/extensions/common/wasm/wasm.h b/source/extensions/common/wasm/wasm.h index a812d1a1a522..882b7c8642b3 100644 --- a/source/extensions/common/wasm/wasm.h +++ b/source/extensions/common/wasm/wasm.h @@ -54,8 +54,8 @@ class Wasm : public WasmBase, Logger::Loggable { Upstream::ClusterManager& clusterManager() const { return cluster_manager_; } Event::Dispatcher& dispatcher() { return dispatcher_; } - Context* getRootContext(absl::string_view root_id) { - return static_cast(WasmBase::getRootContext(root_id)); + Context* getRootContext(const std::shared_ptr& plugin, bool allow_closed) { + return static_cast(WasmBase::getRootContext(plugin, allow_closed)); } void setTimerPeriod(uint32_t root_context_id, std::chrono::milliseconds period) override; virtual void tickHandler(uint32_t root_context_id); @@ -72,12 +72,13 @@ class Wasm : public WasmBase, Logger::Loggable { void getFunctions() override; // AccessLog::Instance - void log(absl::string_view root_id, const Http::RequestHeaderMap* request_headers, + void log(const PluginSharedPtr& plugin, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers, const StreamInfo::StreamInfo& stream_info); - void onStatsUpdate(absl::string_view root_id, Envoy::Stats::MetricSnapshot& snapshot); + void onStatsUpdate(const PluginSharedPtr& plugin, Envoy::Stats::MetricSnapshot& snapshot); + virtual std::string buildVersion() { return BUILD_VERSION_NUMBER; } void initializeLifecycle(Server::ServerLifecycleNotifier& lifecycle_notifier); diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index c62b06c4102d..241d72bc893b 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -1,14 +1,5 @@ #include "extensions/filters/http/wasm/wasm_filter.h" -#include "envoy/http/codes.h" - -#include "common/buffer/buffer_impl.h" -#include "common/common/assert.h" -#include "common/common/enum_to_int.h" -#include "common/http/header_map_impl.h" -#include "common/http/message_impl.h" -#include "common/http/utility.h" - namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -46,6 +37,16 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was } } +FilterConfig::~FilterConfig() { + if (tls_slot_->get()) { + tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) + -> ThreadLocal::ThreadLocalObjectSharedPtr { + object->asType().wasm()->startShutdown(plugin); + return object; + }); + } +} + } // namespace Wasm } // namespace HttpFilters } // namespace Extensions diff --git a/source/extensions/filters/http/wasm/wasm_filter.h b/source/extensions/filters/http/wasm/wasm_filter.h index 36bfd1503b77..44296a452ef6 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.h +++ b/source/extensions/filters/http/wasm/wasm_filter.h @@ -23,6 +23,7 @@ class FilterConfig : Logger::Loggable { public: FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& proto_config, Server::Configuration::FactoryContext& context); + ~FilterConfig(); std::shared_ptr createFilter() { Wasm* wasm = nullptr; @@ -33,7 +34,7 @@ class FilterConfig : Logger::Loggable { return nullptr; } if (wasm && !root_context_id_) { - root_context_id_ = wasm->getRootContext(plugin_->root_id_)->id(); + root_context_id_ = wasm->getRootContext(plugin_, false)->id(); } return std::make_shared(wasm, root_context_id_, plugin_); } diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index 9d253b675abd..fb6481342a5d 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -1,9 +1,5 @@ #include "extensions/filters/network/wasm/wasm_filter.h" -#include "common/buffer/buffer_impl.h" -#include "common/common/assert.h" -#include "common/common/enum_to_int.h" - namespace Envoy { namespace Extensions { namespace NetworkFilters { @@ -41,6 +37,16 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: } } +FilterConfig::~FilterConfig() { + if (tls_slot_->get()) { + tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) + -> ThreadLocal::ThreadLocalObjectSharedPtr { + object->asType().wasm()->startShutdown(plugin); + return object; + }); + } +} + } // namespace Wasm } // namespace NetworkFilters } // namespace Extensions diff --git a/source/extensions/filters/network/wasm/wasm_filter.h b/source/extensions/filters/network/wasm/wasm_filter.h index 51adbcd7ac0c..20f6f8dae166 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.h +++ b/source/extensions/filters/network/wasm/wasm_filter.h @@ -23,6 +23,7 @@ class FilterConfig : Logger::Loggable { public: FilterConfig(const envoy::extensions::filters::network::wasm::v3::Wasm& proto_config, Server::Configuration::FactoryContext& context); + ~FilterConfig(); std::shared_ptr createFilter() { Wasm* wasm = nullptr; @@ -33,11 +34,12 @@ class FilterConfig : Logger::Loggable { return nullptr; } if (wasm && !root_context_id_) { - root_context_id_ = wasm->getRootContext(plugin_->root_id_)->id(); + root_context_id_ = wasm->getRootContext(plugin_, false)->id(); } return std::make_shared(wasm, root_context_id_, plugin_); } - Envoy::Extensions::Common::Wasm::Wasm* wasm() { + + Envoy::Extensions::Common::Wasm::Wasm* wasmForTest() { return tls_slot_->getTyped().wasm().get(); } diff --git a/source/extensions/stat_sinks/wasm/config.cc b/source/extensions/stat_sinks/wasm/config.cc index ba94937a3b3a..44233a56374f 100644 --- a/source/extensions/stat_sinks/wasm/config.cc +++ b/source/extensions/stat_sinks/wasm/config.cc @@ -22,14 +22,14 @@ WasmSinkFactory::createStatsSink(const Protobuf::Message& proto_config, MessageUtil::downcastAndValidate( proto_config, context.messageValidationContext().staticValidationVisitor()); - auto wasm_sink = std::make_unique(config.config().root_id(), nullptr); - auto plugin = std::make_shared( config.config().name(), config.config().root_id(), config.config().vm_config().vm_id(), config.config().vm_config().runtime(), Common::Wasm::anyToBytes(config.config().configuration()), config.config().fail_open(), envoy::config::core::v3::TrafficDirection::UNSPECIFIED, context.localInfo(), nullptr); + auto wasm_sink = std::make_unique(plugin, nullptr); + auto callback = [&wasm_sink, &context, plugin](Common::Wasm::WasmHandleSharedPtr base_wasm) { if (!base_wasm) { if (plugin->fail_open_) { diff --git a/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h b/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h index 5f2a9b6e13f9..0293ab5d5efc 100644 --- a/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h +++ b/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h @@ -9,15 +9,16 @@ namespace Extensions { namespace StatSinks { namespace Wasm { +using Envoy::Extensions::Common::Wasm::PluginSharedPtr; using Envoy::Extensions::Common::Wasm::WasmHandle; class WasmStatSink : public Stats::Sink { public: - WasmStatSink(absl::string_view root_id, Common::Wasm::WasmHandleSharedPtr singleton) - : root_id_(root_id), singleton_(std::move(singleton)) {} + WasmStatSink(const PluginSharedPtr& plugin, Common::Wasm::WasmHandleSharedPtr singleton) + : plugin_(plugin), singleton_(std::move(singleton)) {} void flush(Stats::MetricSnapshot& snapshot) override { - singleton_->wasm()->onStatsUpdate(root_id_, snapshot); + singleton_->wasm()->onStatsUpdate(plugin_, snapshot); } void setSingleton(Common::Wasm::WasmHandleSharedPtr singleton) { @@ -31,7 +32,7 @@ class WasmStatSink : public Stats::Sink { } private: - std::string root_id_; + Common::Wasm::PluginSharedPtr plugin_; Common::Wasm::WasmHandleSharedPtr singleton_; }; diff --git a/test/extensions/bootstrap/wasm/wasm_test.cc b/test/extensions/bootstrap/wasm/wasm_test.cc index 2befcbd97cba..ecc0a5e8e1bd 100644 --- a/test/extensions/bootstrap/wasm/wasm_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_test.cc @@ -207,7 +207,7 @@ TEST_P(WasmTest, Segv) { auto context = static_cast(wasm_->start(plugin_)); EXPECT_CALL(*context, log_(spdlog::level::err, Eq("before badptr"))); EXPECT_FALSE(wasm_->configure(context, plugin_)); - wasm_->isFailed(); + EXPECT_TRUE(wasm_->isFailed()); } TEST_P(WasmTest, DivByZero) { @@ -219,7 +219,7 @@ TEST_P(WasmTest, DivByZero) { auto context = static_cast(wasm_->start(plugin_)); EXPECT_CALL(*context, log_(spdlog::level::err, Eq("before div by zero"))); context->onLog(); - wasm_->isFailed(); + EXPECT_TRUE(wasm_->isFailed()); } TEST_P(WasmTest, IntrinsicGlobals) { diff --git a/test/extensions/common/wasm/test_data/test_cpp.cc b/test/extensions/common/wasm/test_data/test_cpp.cc index 1d990901846a..f2003072ee8c 100644 --- a/test/extensions/common/wasm/test_data/test_cpp.cc +++ b/test/extensions/common/wasm/test_data/test_cpp.cc @@ -267,6 +267,10 @@ WASM_EXPORT(uint32_t, proxy_on_done, (uint32_t)) { return 0; } +WASM_EXPORT(void, proxy_on_tick, (uint32_t)) { + proxy_done(); +} + WASM_EXPORT(void, proxy_on_delete, (uint32_t)) { std::string message = "on_delete logging"; proxy_log(LogLevel::info, message.c_str(), message.size()); diff --git a/test/extensions/common/wasm/wasm_test.cc b/test/extensions/common/wasm/wasm_test.cc index 0c84105cd778..4d656f112aab 100644 --- a/test/extensions/common/wasm/wasm_test.cc +++ b/test/extensions/common/wasm/wasm_test.cc @@ -234,8 +234,8 @@ TEST_P(WasmCommonTest, Logging) { wasm_handle.reset(); dispatcher->run(Event::Dispatcher::RunType::NonBlock); // This will fault on nullptr if wasm has been deleted. - plugin->plugin_configuration_ = "done"; - wasm_weak.lock()->configure(root_context, plugin); + wasm_weak.lock()->setTimerPeriod(root_context->id(), std::chrono::milliseconds(10)); + wasm_weak.lock()->tickHandler(root_context->id()); dispatcher->run(Event::Dispatcher::RunType::NonBlock); dispatcher->clearDeferredDeleteList(); } @@ -645,7 +645,7 @@ TEST_P(WasmCommonTest, VmCache) { auto root_id = ""; auto vm_id = ""; auto vm_configuration = "vm_cache"; - auto plugin_configuration = "init"; + auto plugin_configuration = "done"; auto plugin = std::make_shared( name, root_id, vm_id, GetParam(), plugin_configuration, false, envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info, nullptr); @@ -698,7 +698,6 @@ TEST_P(WasmCommonTest, VmCache) { nullptr, [](Wasm* wasm, const std::shared_ptr& plugin) -> ContextBase* { auto root_context = new TestContext(wasm, plugin); EXPECT_CALL(*root_context, log_(spdlog::level::info, Eq("on_vm_start vm_cache"))); - EXPECT_CALL(*root_context, log_(spdlog::level::info, Eq("on_configuration init"))); EXPECT_CALL(*root_context, log_(spdlog::level::info, Eq("on_done logging"))); EXPECT_CALL(*root_context, log_(spdlog::level::info, Eq("on_delete logging"))); return root_context; @@ -708,12 +707,10 @@ TEST_P(WasmCommonTest, VmCache) { wasm_handle.reset(); wasm_handle2.reset(); - auto wasm = wasm_handle_local->wasm().get(); + auto wasm = wasm_handle_local->wasm(); wasm_handle_local.reset(); dispatcher->run(Event::Dispatcher::RunType::NonBlock); - - plugin->plugin_configuration_ = "done"; wasm->configure(wasm->getContext(1), plugin); plugin.reset(); dispatcher->run(Event::Dispatcher::RunType::NonBlock); @@ -809,7 +806,7 @@ TEST_P(WasmCommonTest, RemoteCode) { }); wasm_handle.reset(); - auto wasm = wasm_handle_local->wasm().get(); + auto wasm = wasm_handle_local->wasm(); wasm_handle_local.reset(); dispatcher->run(Event::Dispatcher::RunType::NonBlock); wasm->configure(wasm->getContext(1), plugin); @@ -919,7 +916,7 @@ TEST_P(WasmCommonTest, RemoteCodeMultipleRetry) { }); wasm_handle.reset(); - auto wasm = wasm_handle_local->wasm().get(); + auto wasm = wasm_handle_local->wasm(); wasm_handle_local.reset(); dispatcher->run(Event::Dispatcher::RunType::NonBlock); diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc index 538481f36f6c..ba6819f1d8b3 100644 --- a/test/extensions/filters/http/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc @@ -82,7 +82,7 @@ class WasmHttpFilterTest : public Common::Wasm::WasmHttpFilterTestBase< } setupBase(std::get<0>(GetParam()), code, createContextFn(), root_id, vm_configuration); } - void setupFilter(const std::string root_id = "") { setupFilterBase(root_id); } + void setupFilter() { setupFilterBase(); } void setupGrpcStreamTest(Grpc::RawAsyncStreamCallbacks*& callbacks); @@ -291,7 +291,7 @@ TEST_P(WasmHttpFilterTest, HeadersStopAndWatermark) { // Script that reads the body. TEST_P(WasmHttpFilterTest, BodyRequestReadBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::err, Eq(absl::string_view("onBody hello")))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"x-test-operation", "ReadBody"}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, false)); @@ -303,7 +303,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestReadBody) { // Script that prepends and appends to the body. TEST_P(WasmHttpFilterTest, BodyRequestPrependAndAppendToBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::err, Eq(absl::string_view("onBody prepend.hello.append")))); EXPECT_CALL(filter(), log_(spdlog::level::err, @@ -327,7 +327,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestPrependAndAppendToBody) { // Script that replaces the body. TEST_P(WasmHttpFilterTest, BodyRequestReplaceBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::err, Eq(absl::string_view("onBody replace")))).Times(2); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"x-test-operation", "ReplaceBody"}}; @@ -348,7 +348,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestReplaceBody) { // Script that removes the body. TEST_P(WasmHttpFilterTest, BodyRequestRemoveBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::err, Eq(absl::string_view("onBody ")))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"x-test-operation", "RemoveBody"}}; @@ -361,7 +361,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestRemoveBody) { // Script that buffers the body. TEST_P(WasmHttpFilterTest, BodyRequestBufferBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"x-test-operation", "BufferBody"}}; @@ -404,7 +404,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestBufferBody) { // Script that prepends and appends to the buffered body. TEST_P(WasmHttpFilterTest, BodyRequestPrependAndAppendToBufferedBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::err, Eq(absl::string_view("onBody prepend.hello.append")))); Http::TestRequestHeaderMapImpl request_headers{ @@ -418,7 +418,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestPrependAndAppendToBufferedBody) { // Script that replaces the buffered body. TEST_P(WasmHttpFilterTest, BodyRequestReplaceBufferedBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::err, Eq(absl::string_view("onBody replace")))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"x-test-operation", "ReplaceBufferedBody"}}; @@ -431,7 +431,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestReplaceBufferedBody) { // Script that removes the buffered body. TEST_P(WasmHttpFilterTest, BodyRequestRemoveBufferedBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::err, Eq(absl::string_view("onBody ")))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"x-test-operation", "RemoveBufferedBody"}}; @@ -444,7 +444,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestRemoveBufferedBody) { // Script that buffers the first part of the body and streams the rest TEST_P(WasmHttpFilterTest, BodyRequestBufferThenStreamBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, false)); @@ -494,7 +494,7 @@ TEST_P(WasmHttpFilterTest, BodyRequestBufferThenStreamBody) { // Script that buffers the first part of the body and streams the rest TEST_P(WasmHttpFilterTest, BodyResponseBufferThenStreamBody) { setupTest("body"); - setupFilter("body"); + setupFilter(); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, false)); @@ -577,7 +577,7 @@ TEST_P(WasmHttpFilterTest, AccessLogCreate) { TEST_P(WasmHttpFilterTest, AsyncCall) { setupTest("async_call"); - setupFilter("async_call"); + setupFilter(); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); @@ -624,7 +624,7 @@ TEST_P(WasmHttpFilterTest, AsyncCallBadCall) { return; } setupTest("async_call"); - setupFilter("async_call"); + setupFilter(); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); @@ -644,7 +644,7 @@ TEST_P(WasmHttpFilterTest, AsyncCallBadCall) { TEST_P(WasmHttpFilterTest, AsyncCallFailure) { setupTest("async_call"); - setupFilter("async_call"); + setupFilter(); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); @@ -685,7 +685,7 @@ TEST_P(WasmHttpFilterTest, AsyncCallFailure) { TEST_P(WasmHttpFilterTest, AsyncCallAfterDestroyed) { setupTest("async_call"); - setupFilter("async_call"); + setupFilter(); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); @@ -735,7 +735,7 @@ TEST_P(WasmHttpFilterTest, GrpcCall) { return; } setupTest("grpc_call"); - setupFilter("grpc_call"); + setupFilter(); NiceMock request; Grpc::RawAsyncRequestCallbacks* callbacks = nullptr; Grpc::MockAsyncClientManager client_manager; @@ -790,7 +790,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallBadCall) { return; } setupTest("grpc_call"); - setupFilter("grpc_call"); + setupFilter(); Grpc::MockAsyncClientManager client_manager; auto client_factory = std::make_unique(); auto async_client = std::make_unique(); @@ -819,7 +819,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallFailure) { return; } setupTest("grpc_call"); - setupFilter("grpc_call"); + setupFilter(); NiceMock request; Grpc::RawAsyncRequestCallbacks* callbacks = nullptr; Grpc::MockAsyncClientManager client_manager; @@ -881,7 +881,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallCancel) { return; } setupTest("grpc_call"); - setupFilter("grpc_call"); + setupFilter(); NiceMock request; Grpc::RawAsyncRequestCallbacks* callbacks = nullptr; Grpc::MockAsyncClientManager client_manager; @@ -925,7 +925,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallClose) { return; } setupTest("grpc_call"); - setupFilter("grpc_call"); + setupFilter(); NiceMock request; Grpc::RawAsyncRequestCallbacks* callbacks = nullptr; Grpc::MockAsyncClientManager client_manager; @@ -969,7 +969,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallAfterDestroyed) { return; } setupTest("grpc_call"); - setupFilter("grpc_call"); + setupFilter(); Grpc::MockAsyncRequest request; Grpc::RawAsyncRequestCallbacks* callbacks = nullptr; Grpc::MockAsyncClientManager client_manager; @@ -1026,7 +1026,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallAfterDestroyed) { void WasmHttpFilterTest::setupGrpcStreamTest(Grpc::RawAsyncStreamCallbacks*& callbacks) { setupTest("grpc_stream"); - setupFilter("grpc_stream"); + setupFilter(); EXPECT_CALL(async_client_manager_, factoryForGrpcService(_, _, _)) .WillRepeatedly( @@ -1381,7 +1381,7 @@ TEST_P(WasmHttpFilterTest, SharedData) { TEST_P(WasmHttpFilterTest, SharedQueue) { setupTest("shared_queue"); - setupFilter("shared_queue"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::warn, Eq(absl::string_view("onRequestHeaders enqueue Ok")))); EXPECT_CALL(filter(), log_(spdlog::level::warn, @@ -1420,7 +1420,7 @@ TEST_P(WasmHttpFilterTest, RootId1) { return; } setupTest("context1"); - setupFilter("context1"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::debug, Eq(absl::string_view("onRequestHeaders1 2")))); Http::TestRequestHeaderMapImpl request_headers; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, true)); @@ -1433,7 +1433,7 @@ TEST_P(WasmHttpFilterTest, RootId2) { return; } setupTest("context2"); - setupFilter("context2"); + setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::debug, Eq(absl::string_view("onRequestHeaders2 2")))); Http::TestRequestHeaderMapImpl request_headers; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, true)); diff --git a/test/extensions/filters/network/wasm/config_test.cc b/test/extensions/filters/network/wasm/config_test.cc index 58d17c177fb7..257bb52ce5ff 100644 --- a/test/extensions/filters/network/wasm/config_test.cc +++ b/test/extensions/filters/network/wasm/config_test.cc @@ -180,7 +180,7 @@ TEST_P(WasmNetworkFilterConfigTest, FilterConfigFailOpen) { envoy::extensions::filters::network::wasm::v3::Wasm proto_config; TestUtility::loadFromYaml(yaml, proto_config); NetworkFilters::Wasm::FilterConfig filter_config(proto_config, context_); - filter_config.wasm()->fail(proxy_wasm::FailState::RuntimeError, ""); + filter_config.wasmForTest()->fail(proxy_wasm::FailState::RuntimeError, ""); EXPECT_EQ(filter_config.createFilter(), nullptr); } diff --git a/test/extensions/filters/network/wasm/wasm_filter_test.cc b/test/extensions/filters/network/wasm/wasm_filter_test.cc index 6bf1ca8151e6..98e578e68c2d 100644 --- a/test/extensions/filters/network/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/network/wasm/wasm_filter_test.cc @@ -58,7 +58,7 @@ class WasmNetworkFilterTest : public Common::Wasm::WasmNetworkFilterTestBase< "" /* root_id */, "" /* vm_configuration */, fail_open); } - void setupFilter() { setupFilterBase(""); } + void setupFilter() { setupFilterBase(); } TestFilter& filter() { return *static_cast(context_.get()); } diff --git a/test/test_common/wasm_base.h b/test/test_common/wasm_base.h index d049460d3e41..a8231c0c1ca2 100644 --- a/test/test_common/wasm_base.h +++ b/test/test_common/wasm_base.h @@ -114,9 +114,9 @@ template class WasmTestBase : public Base { template class WasmHttpFilterTestBase : public WasmTestBase { public: - template void setupFilterBase(const std::string root_id = "") { + template void setupFilterBase() { auto wasm = WasmTestBase::wasm_ ? WasmTestBase::wasm_->wasm().get() : nullptr; - int root_context_id = wasm ? wasm->getRootContext(root_id)->id() : 0; + int root_context_id = wasm ? wasm->getRootContext(WasmTestBase::plugin_, false)->id() : 0; context_ = std::make_unique(wasm, root_context_id, WasmTestBase::plugin_); context_->setDecoderFilterCallbacks(decoder_callbacks_); context_->setEncoderFilterCallbacks(encoder_callbacks_); @@ -131,9 +131,9 @@ template class WasmHttpFilterTestBase : public W template class WasmNetworkFilterTestBase : public WasmTestBase { public: - template void setupFilterBase(const std::string root_id = "") { + template void setupFilterBase() { auto wasm = WasmTestBase::wasm_ ? WasmTestBase::wasm_->wasm().get() : nullptr; - int root_context_id = wasm ? wasm->getRootContext(root_id)->id() : 0; + int root_context_id = wasm ? wasm->getRootContext(WasmTestBase::plugin_, false)->id() : 0; context_ = std::make_unique(wasm, root_context_id, WasmTestBase::plugin_); context_->initializeReadFilterCallbacks(read_filter_callbacks_); context_->initializeWriteFilterCallbacks(write_filter_callbacks_); From 672c1ff55c6d20df579a12f5ad462f7035465c7b Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 29 Oct 2020 07:41:53 +0000 Subject: [PATCH 02/15] review: fix clang-tidy. Signed-off-by: Piotr Sikora --- source/extensions/access_loggers/wasm/wasm_access_log_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index c0467fba790c..e47e9ba06427 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -43,7 +43,7 @@ class WasmAccessLog : public AccessLog::Instance { tls_slot_ = std::move(tls_slot); } - ~WasmAccessLog() { + ~WasmAccessLog() override { if (tls_slot_->get()) { tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) -> ThreadLocal::ThreadLocalObjectSharedPtr { From 8d49c1d56e893efa524a2160cab93842ef4c5fb2 Mon Sep 17 00:00:00 2001 From: mathetake Date: Fri, 30 Oct 2020 15:49:40 +0900 Subject: [PATCH 03/15] wasm: remove the gap between rust and cpp test code for logging Signed-off-by: mathetake --- test/extensions/bootstrap/wasm/test_data/logging_cpp.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/extensions/bootstrap/wasm/test_data/logging_cpp.cc b/test/extensions/bootstrap/wasm/test_data/logging_cpp.cc index 70fde8f6ae19..9a5becfab1b3 100644 --- a/test/extensions/bootstrap/wasm/test_data/logging_cpp.cc +++ b/test/extensions/bootstrap/wasm/test_data/logging_cpp.cc @@ -26,10 +26,7 @@ extern "C" PROXY_WASM_KEEPALIVE uint32_t proxy_on_configure(uint32_t, uint32_t c extern "C" PROXY_WASM_KEEPALIVE void proxy_on_context_create(uint32_t, uint32_t) {} -extern "C" PROXY_WASM_KEEPALIVE uint32_t proxy_on_vm_start(uint32_t, uint32_t) { - proxy_set_tick_period_milliseconds(10); - return 1; -} +extern "C" PROXY_WASM_KEEPALIVE uint32_t proxy_on_vm_start(uint32_t, uint32_t) { return 1; } extern "C" PROXY_WASM_KEEPALIVE void proxy_on_tick(uint32_t) { const char* root_id = nullptr; From 9f98a40e6a1ce9cbeb1a796e8ac91cc9e193dd9e Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Fri, 30 Oct 2020 08:47:19 +0000 Subject: [PATCH 04/15] review: use merged commit. Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 06ffb0737064..bf5ed63b361d 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -830,10 +830,10 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "fa18f0005afd47e515577af7fdfe94b8a0268860", - sha256 = "8dfb533513fbc5bb2493bae1b8dc825bcc2c31776c6b1761b1713ac1f133df5a", + version = "f08baacadbe656674414f0878e18852bed67b795", + sha256 = "dc3239c674d19198f71becf5d1c2bf159128af3c72ec2f9c532b3153ecb6d5a1", strip_prefix = "proxy-wasm-cpp-host-{version}", - urls = ["https://github.com/PiotrSikora/proxy-wasm-cpp-host/archive/{version}.tar.gz"], + urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], extensions = [ "envoy.access_loggers.wasm", @@ -842,7 +842,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - release_date = "2020-10-29", + release_date = "2020-10-30", cpe = "N/A", ), emscripten_toolchain = dict( From 33f8b48fdd836ab05d3852f2223dcbdf78ac5ca3 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Mon, 2 Nov 2020 04:52:14 +0000 Subject: [PATCH 05/15] review: mark destructors virtual. Signed-off-by: Piotr Sikora --- source/extensions/bootstrap/wasm/config.h | 2 +- source/extensions/filters/http/wasm/wasm_filter.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 6cc4494e46da..31239b961755 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -22,7 +22,7 @@ class WasmService { WasmService(Common::Wasm::PluginSharedPtr plugin, ThreadLocal::SlotPtr tls_slot) : plugin_(plugin), tls_slot_(std::move(tls_slot)) {} - ~WasmService() { + virtual ~WasmService() { if (tls_slot_ && tls_slot_->get()) { tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) -> ThreadLocal::ThreadLocalObjectSharedPtr { diff --git a/source/extensions/filters/http/wasm/wasm_filter.h b/source/extensions/filters/http/wasm/wasm_filter.h index 44296a452ef6..02e99674305f 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.h +++ b/source/extensions/filters/http/wasm/wasm_filter.h @@ -23,7 +23,7 @@ class FilterConfig : Logger::Loggable { public: FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& proto_config, Server::Configuration::FactoryContext& context); - ~FilterConfig(); + virtual ~FilterConfig(); std::shared_ptr createFilter() { Wasm* wasm = nullptr; From 375b02d62abf3fa001dd57597e5bdd5e72364849 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Mon, 2 Nov 2020 04:54:42 +0000 Subject: [PATCH 06/15] reivew: use tls_slot_->currentThreadRegistered(). Signed-off-by: Piotr Sikora --- source/extensions/access_loggers/wasm/wasm_access_log_impl.h | 4 ++-- source/extensions/bootstrap/wasm/config.h | 2 +- source/extensions/filters/http/wasm/wasm_filter.cc | 2 +- source/extensions/filters/network/wasm/wasm_filter.cc | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index e47e9ba06427..fa23e204695a 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -32,7 +32,7 @@ class WasmAccessLog : public AccessLog::Instance { } } - if (tls_slot_->get()) { + if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { tls_slot_->getTyped().wasm()->log(plugin_, request_headers, response_headers, response_trailers, stream_info); } @@ -44,7 +44,7 @@ class WasmAccessLog : public AccessLog::Instance { } ~WasmAccessLog() override { - if (tls_slot_->get()) { + if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) -> ThreadLocal::ThreadLocalObjectSharedPtr { object->asType().wasm()->startShutdown(plugin); diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 31239b961755..b0daacd1683e 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -23,7 +23,7 @@ class WasmService { : plugin_(plugin), tls_slot_(std::move(tls_slot)) {} virtual ~WasmService() { - if (tls_slot_ && tls_slot_->get()) { + if (tls_slot_ && tls_slot_->currentThreadRegistered() && tls_slot_->get()) { tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) -> ThreadLocal::ThreadLocalObjectSharedPtr { object->asType().wasm()->startShutdown(plugin); diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index 241d72bc893b..3552bf6410ad 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -38,7 +38,7 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was } FilterConfig::~FilterConfig() { - if (tls_slot_->get()) { + if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) -> ThreadLocal::ThreadLocalObjectSharedPtr { object->asType().wasm()->startShutdown(plugin); diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index fb6481342a5d..b33f129b97d4 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -38,7 +38,7 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: } FilterConfig::~FilterConfig() { - if (tls_slot_->get()) { + if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) -> ThreadLocal::ThreadLocalObjectSharedPtr { object->asType().wasm()->startShutdown(plugin); From 3a89a49aa902709d71ac66b74506a0a22f61627e Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Tue, 3 Nov 2020 03:47:39 +0000 Subject: [PATCH 07/15] review: use ThreadLocal::TypedSlot. Signed-off-by: Piotr Sikora --- .../extensions/access_loggers/wasm/config.cc | 21 +++----------- .../wasm/wasm_access_log_impl.h | 24 ++++++++-------- source/extensions/bootstrap/wasm/config.cc | 5 ++-- source/extensions/bootstrap/wasm/config.h | 17 ++++++----- source/extensions/common/wasm/wasm.cc | 8 ++++++ .../filters/http/wasm/wasm_filter.cc | 26 ++++++----------- .../filters/http/wasm/wasm_filter.h | 6 ++-- .../filters/network/wasm/wasm_filter.cc | 28 ++++++++----------- .../filters/network/wasm/wasm_filter.h | 10 +++---- 9 files changed, 62 insertions(+), 83 deletions(-) diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc index 800b6484c93c..a0472415fed2 100644 --- a/source/extensions/access_loggers/wasm/config.cc +++ b/source/extensions/access_loggers/wasm/config.cc @@ -36,24 +36,11 @@ WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_con auto access_log = std::make_shared(plugin, nullptr, std::move(filter)); auto callback = [access_log, &context, plugin](Common::Wasm::WasmHandleSharedPtr base_wasm) { - auto tls_slot = context.threadLocal().allocateSlot(); - // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. - tls_slot->set( - [base_wasm, - plugin](Event::Dispatcher& dispatcher) -> std::shared_ptr { - if (!base_wasm) { - // There is no way to prevent the connection at this point. The user could choose to use - // an HTTP Wasm plugin and only handle onLog() which would correctly close the - // connection in onRequestHeaders(). - if (!plugin->fail_open_) { - ENVOY_LOG(critical, "Plugin configured to fail closed failed to load"); - } - return nullptr; - } - return std::static_pointer_cast( - Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher)); - }); + auto tls_slot = ThreadLocal::TypedSlot::makeUnique(context.threadLocal()); + tls_slot->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { + return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); + }); access_log->setTlsSlot(std::move(tls_slot)); }; diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index fa23e204695a..dd82098ca46d 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -17,7 +17,7 @@ using Envoy::Extensions::Common::Wasm::WasmHandle; class WasmAccessLog : public AccessLog::Instance { public: - WasmAccessLog(const PluginSharedPtr& plugin, ThreadLocal::SlotPtr tls_slot, + WasmAccessLog(const PluginSharedPtr& plugin, ThreadLocal::TypedSlotPtr&& tls_slot, AccessLog::FilterPtr filter) : plugin_(plugin), tls_slot_(std::move(tls_slot)), filter_(std::move(filter)) {} @@ -32,30 +32,28 @@ class WasmAccessLog : public AccessLog::Instance { } } - if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { - tls_slot_->getTyped().wasm()->log(plugin_, request_headers, response_headers, - response_trailers, stream_info); + if (tls_slot_->get().wasm()) { + tls_slot_->get().wasm()->log(plugin_, request_headers, response_headers, response_trailers, + stream_info); } } - void setTlsSlot(ThreadLocal::SlotPtr tls_slot) { + void setTlsSlot(ThreadLocal::TypedSlotPtr&& tls_slot) { ASSERT(tls_slot_ == nullptr); tls_slot_ = std::move(tls_slot); } ~WasmAccessLog() override { - if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { - tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) - -> ThreadLocal::ThreadLocalObjectSharedPtr { - object->asType().wasm()->startShutdown(plugin); - return object; - }); - } + tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { + if (handle.wasm()) { + handle.wasm()->startShutdown(plugin); + } + }); } private: Common::Wasm::PluginSharedPtr plugin_; - ThreadLocal::SlotPtr tls_slot_; + ThreadLocal::TypedSlotPtr tls_slot_; AccessLog::FilterPtr filter_; }; diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 3d9b9fd1d799..ed21ffdc3ea5 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -43,10 +43,9 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con } // Per-thread WASM VM. // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. - auto tls_slot = context.threadLocal().allocateSlot(); + auto tls_slot = ThreadLocal::TypedSlot::makeUnique(context.threadLocal()); tls_slot->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { - return std::static_pointer_cast( - Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher)); + return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); }); cb(std::make_unique(plugin, std::move(tls_slot))); }; diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index b0daacd1683e..29e5d136b618 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -16,18 +16,21 @@ namespace Extensions { namespace Bootstrap { namespace Wasm { +using Envoy::Extensions::Common::Wasm::WasmHandle; + class WasmService { public: WasmService(Common::Wasm::WasmHandleSharedPtr singleton) : singleton_(std::move(singleton)) {} - WasmService(Common::Wasm::PluginSharedPtr plugin, ThreadLocal::SlotPtr tls_slot) + WasmService(Common::Wasm::PluginSharedPtr plugin, + ThreadLocal::TypedSlotPtr&& tls_slot) : plugin_(plugin), tls_slot_(std::move(tls_slot)) {} virtual ~WasmService() { - if (tls_slot_ && tls_slot_->currentThreadRegistered() && tls_slot_->get()) { - tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) - -> ThreadLocal::ThreadLocalObjectSharedPtr { - object->asType().wasm()->startShutdown(plugin); - return object; + if (!singleton_) { + tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { + if (handle.wasm()) { + handle.wasm()->startShutdown(plugin); + } }); } } @@ -35,7 +38,7 @@ class WasmService { private: Common::Wasm::PluginSharedPtr plugin_; Common::Wasm::WasmHandleSharedPtr singleton_; - ThreadLocal::SlotPtr tls_slot_; + ThreadLocal::TypedSlotPtr tls_slot_; }; using WasmServicePtr = std::unique_ptr; diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index 7a2a2478e82a..706e389d433a 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -478,6 +478,14 @@ WasmHandleSharedPtr getOrCreateThreadLocalWasm(const WasmHandleSharedPtr& base_w const PluginSharedPtr& plugin, Event::Dispatcher& dispatcher, CreateContextFn create_root_context_for_testing) { + if (!base_wasm || !base_wasm->wasm()) { + if (!plugin->fail_open_) { + ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::wasm), critical, + "Plugin configured to fail closed failed to load"); + } + // ThreadLocalObject cannot be nullptr, so create a WasmHandle holding nullptr. + return std::make_shared(nullptr); + } return std::static_pointer_cast(proxy_wasm::getOrCreateThreadLocalWasm( std::static_pointer_cast(base_wasm), plugin, getCloneFactory(getWasmExtension(), dispatcher, create_root_context_for_testing))); diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index 3552bf6410ad..d43889cd8eaf 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -7,7 +7,7 @@ namespace Wasm { FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, Server::Configuration::FactoryContext& context) - : tls_slot_(context.threadLocal().allocateSlot()) { + : tls_slot_(ThreadLocal::TypedSlot::makeUnique(context.threadLocal())) { plugin_ = std::make_shared( config.config().name(), config.config().root_id(), config.config().vm_config().vm_id(), config.config().vm_config().runtime(), @@ -17,15 +17,9 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was auto plugin = plugin_; auto callback = [plugin, this](const Common::Wasm::WasmHandleSharedPtr& base_wasm) { // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. - tls_slot_->set( - [base_wasm, - plugin](Event::Dispatcher& dispatcher) -> std::shared_ptr { - if (!base_wasm) { - return nullptr; - } - return std::static_pointer_cast( - Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher)); - }); + tls_slot_->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { + return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); + }); }; if (!Common::Wasm::createWasm( @@ -38,13 +32,11 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was } FilterConfig::~FilterConfig() { - if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { - tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) - -> ThreadLocal::ThreadLocalObjectSharedPtr { - object->asType().wasm()->startShutdown(plugin); - return object; - }); - } + tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { + if (handle.wasm()) { + handle.wasm()->startShutdown(plugin); + } + }); } } // namespace Wasm diff --git a/source/extensions/filters/http/wasm/wasm_filter.h b/source/extensions/filters/http/wasm/wasm_filter.h index 02e99674305f..29f9b7cb65ab 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.h +++ b/source/extensions/filters/http/wasm/wasm_filter.h @@ -27,8 +27,8 @@ class FilterConfig : Logger::Loggable { std::shared_ptr createFilter() { Wasm* wasm = nullptr; - if (tls_slot_->get()) { - wasm = tls_slot_->getTyped().wasm().get(); + if (tls_slot_->get().wasm()) { + wasm = tls_slot_->get().wasm().get(); } if (plugin_->fail_open_ && (!wasm || wasm->isFailed())) { return nullptr; @@ -42,7 +42,7 @@ class FilterConfig : Logger::Loggable { private: uint32_t root_context_id_{0}; Envoy::Extensions::Common::Wasm::PluginSharedPtr plugin_; - ThreadLocal::SlotPtr tls_slot_; + ThreadLocal::TypedSlotPtr tls_slot_; Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_; }; diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index b33f129b97d4..d0d3bc5a6577 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -5,9 +5,11 @@ namespace Extensions { namespace NetworkFilters { namespace Wasm { +using Envoy::Extensions::Common::Wasm::WasmHandle; + FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3::Wasm& config, Server::Configuration::FactoryContext& context) - : tls_slot_(context.threadLocal().allocateSlot()) { + : tls_slot_(ThreadLocal::TypedSlot::makeUnique(context.threadLocal())) { plugin_ = std::make_shared( config.config().name(), config.config().root_id(), config.config().vm_config().vm_id(), config.config().vm_config().runtime(), @@ -17,15 +19,9 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: auto plugin = plugin_; auto callback = [plugin, this](Common::Wasm::WasmHandleSharedPtr base_wasm) { // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. - tls_slot_->set( - [base_wasm, - plugin](Event::Dispatcher& dispatcher) -> std::shared_ptr { - if (!base_wasm) { - return nullptr; - } - return std::static_pointer_cast( - Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher)); - }); + tls_slot_->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { + return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); + }); }; if (!Common::Wasm::createWasm( @@ -38,13 +34,11 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: } FilterConfig::~FilterConfig() { - if (tls_slot_->currentThreadRegistered() && tls_slot_->get()) { - tls_slot_->runOnAllThreads([plugin = plugin_](ThreadLocal::ThreadLocalObjectSharedPtr object) - -> ThreadLocal::ThreadLocalObjectSharedPtr { - object->asType().wasm()->startShutdown(plugin); - return object; - }); - } + tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { + if (handle.wasm()) { + handle.wasm()->startShutdown(plugin); + } + }); } } // namespace Wasm diff --git a/source/extensions/filters/network/wasm/wasm_filter.h b/source/extensions/filters/network/wasm/wasm_filter.h index 20f6f8dae166..436709b02481 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.h +++ b/source/extensions/filters/network/wasm/wasm_filter.h @@ -27,8 +27,8 @@ class FilterConfig : Logger::Loggable { std::shared_ptr createFilter() { Wasm* wasm = nullptr; - if (tls_slot_->get()) { - wasm = tls_slot_->getTyped().wasm().get(); + if (tls_slot_->get().wasm()) { + wasm = tls_slot_->get().wasm().get(); } if (plugin_->fail_open_ && (!wasm || wasm->isFailed())) { return nullptr; @@ -39,14 +39,12 @@ class FilterConfig : Logger::Loggable { return std::make_shared(wasm, root_context_id_, plugin_); } - Envoy::Extensions::Common::Wasm::Wasm* wasmForTest() { - return tls_slot_->getTyped().wasm().get(); - } + Envoy::Extensions::Common::Wasm::Wasm* wasmForTest() { return tls_slot_->get().wasm().get(); } private: uint32_t root_context_id_{0}; Envoy::Extensions::Common::Wasm::PluginSharedPtr plugin_; - ThreadLocal::SlotPtr tls_slot_; + ThreadLocal::TypedSlotPtr tls_slot_; Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_; }; From 2facb4189a7a6bdb16f00a9574c17d377c0cf4e7 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Tue, 3 Nov 2020 03:54:34 +0000 Subject: [PATCH 08/15] review: re-add currentThreadRegistered() check. Signed-off-by: Piotr Sikora --- .../access_loggers/wasm/wasm_access_log_impl.h | 12 +++++++----- source/extensions/bootstrap/wasm/config.h | 2 +- source/extensions/filters/http/wasm/wasm_filter.cc | 12 +++++++----- .../extensions/filters/network/wasm/wasm_filter.cc | 12 +++++++----- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index dd82098ca46d..a62bba5fcc25 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -44,11 +44,13 @@ class WasmAccessLog : public AccessLog::Instance { } ~WasmAccessLog() override { - tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { - if (handle.wasm()) { - handle.wasm()->startShutdown(plugin); - } - }); + if (tls_slot_->currentThreadRegistered()) { + tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { + if (handle.wasm()) { + handle.wasm()->startShutdown(plugin); + } + }); + } } private: diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 29e5d136b618..50b9d3b1e760 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -26,7 +26,7 @@ class WasmService { : plugin_(plugin), tls_slot_(std::move(tls_slot)) {} virtual ~WasmService() { - if (!singleton_) { + if (!singleton_ && tls_slot_->currentThreadRegistered()) { tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { if (handle.wasm()) { handle.wasm()->startShutdown(plugin); diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index d43889cd8eaf..e47e68b394cd 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -32,11 +32,13 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was } FilterConfig::~FilterConfig() { - tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { - if (handle.wasm()) { - handle.wasm()->startShutdown(plugin); - } - }); + if (tls_slot_->currentThreadRegistered()) { + tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { + if (handle.wasm()) { + handle.wasm()->startShutdown(plugin); + } + }); + } } } // namespace Wasm diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index d0d3bc5a6577..e4bbf17c35d7 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -34,11 +34,13 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: } FilterConfig::~FilterConfig() { - tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { - if (handle.wasm()) { - handle.wasm()->startShutdown(plugin); - } - }); + if (tls_slot_->currentThreadRegistered()) { + tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { + if (handle.wasm()) { + handle.wasm()->startShutdown(plugin); + } + }); + } } } // namespace Wasm From 4c0ed76c3cae6d69b77b81da6da5c1fd7392d3c5 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Wed, 4 Nov 2020 01:55:10 +0000 Subject: [PATCH 09/15] review: style. Signed-off-by: Piotr Sikora --- .../extensions/access_loggers/wasm/wasm_access_log_impl.h | 6 +++--- source/extensions/filters/http/wasm/wasm_filter.h | 7 ++----- source/extensions/filters/network/wasm/wasm_filter.h | 7 ++----- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index a62bba5fcc25..a783a90b583f 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -32,9 +32,9 @@ class WasmAccessLog : public AccessLog::Instance { } } - if (tls_slot_->get().wasm()) { - tls_slot_->get().wasm()->log(plugin_, request_headers, response_headers, response_trailers, - stream_info); + auto wasm = tls_slot_->get().wasm(); + if (wasm) { + wasm->log(plugin_, request_headers, response_headers, response_trailers, stream_info); } } diff --git a/source/extensions/filters/http/wasm/wasm_filter.h b/source/extensions/filters/http/wasm/wasm_filter.h index 29f9b7cb65ab..bb37e240b8ac 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.h +++ b/source/extensions/filters/http/wasm/wasm_filter.h @@ -26,17 +26,14 @@ class FilterConfig : Logger::Loggable { virtual ~FilterConfig(); std::shared_ptr createFilter() { - Wasm* wasm = nullptr; - if (tls_slot_->get().wasm()) { - wasm = tls_slot_->get().wasm().get(); - } + auto wasm = tls_slot_->get().wasm(); if (plugin_->fail_open_ && (!wasm || wasm->isFailed())) { return nullptr; } if (wasm && !root_context_id_) { root_context_id_ = wasm->getRootContext(plugin_, false)->id(); } - return std::make_shared(wasm, root_context_id_, plugin_); + return std::make_shared(wasm.get(), root_context_id_, plugin_); } private: diff --git a/source/extensions/filters/network/wasm/wasm_filter.h b/source/extensions/filters/network/wasm/wasm_filter.h index 436709b02481..58c57d59bc17 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.h +++ b/source/extensions/filters/network/wasm/wasm_filter.h @@ -26,17 +26,14 @@ class FilterConfig : Logger::Loggable { ~FilterConfig(); std::shared_ptr createFilter() { - Wasm* wasm = nullptr; - if (tls_slot_->get().wasm()) { - wasm = tls_slot_->get().wasm().get(); - } + auto wasm = tls_slot_->get().wasm(); if (plugin_->fail_open_ && (!wasm || wasm->isFailed())) { return nullptr; } if (wasm && !root_context_id_) { root_context_id_ = wasm->getRootContext(plugin_, false)->id(); } - return std::make_shared(wasm, root_context_id_, plugin_); + return std::make_shared(wasm.get(), root_context_id_, plugin_); } Envoy::Extensions::Common::Wasm::Wasm* wasmForTest() { return tls_slot_->get().wasm().get(); } From e94fd8219a53c3b58212f1b524297aa5fd8dcd6f Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Wed, 4 Nov 2020 21:58:38 +0000 Subject: [PATCH 10/15] review: use OptRef. Signed-off-by: Piotr Sikora --- .../access_loggers/wasm/wasm_access_log_impl.h | 13 +++++++------ source/extensions/bootstrap/wasm/config.h | 6 +++--- source/extensions/common/wasm/wasm.cc | 5 ++--- source/extensions/filters/http/wasm/wasm_filter.cc | 6 +++--- source/extensions/filters/http/wasm/wasm_filter.h | 8 ++++++-- .../extensions/filters/network/wasm/wasm_filter.cc | 6 +++--- .../extensions/filters/network/wasm/wasm_filter.h | 10 +++++++--- 7 files changed, 31 insertions(+), 23 deletions(-) diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index a783a90b583f..f1690cfc28ed 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -32,9 +32,10 @@ class WasmAccessLog : public AccessLog::Instance { } } - auto wasm = tls_slot_->get().wasm(); - if (wasm) { - wasm->log(plugin_, request_headers, response_headers, response_trailers, stream_info); + auto handle = tls_slot_->get(); + if (handle.has_value()) { + handle->wasm()->log(plugin_, request_headers, response_headers, response_trailers, + stream_info); } } @@ -45,9 +46,9 @@ class WasmAccessLog : public AccessLog::Instance { ~WasmAccessLog() override { if (tls_slot_->currentThreadRegistered()) { - tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { - if (handle.wasm()) { - handle.wasm()->startShutdown(plugin); + tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { + if (handle.has_value()) { + handle->wasm()->startShutdown(plugin); } }); } diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 50b9d3b1e760..66b331546167 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -27,9 +27,9 @@ class WasmService { virtual ~WasmService() { if (!singleton_ && tls_slot_->currentThreadRegistered()) { - tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { - if (handle.wasm()) { - handle.wasm()->startShutdown(plugin); + tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { + if (handle.has_value()) { + handle->wasm()->startShutdown(plugin); } }); } diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index 706e389d433a..6c939c989e86 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -478,13 +478,12 @@ WasmHandleSharedPtr getOrCreateThreadLocalWasm(const WasmHandleSharedPtr& base_w const PluginSharedPtr& plugin, Event::Dispatcher& dispatcher, CreateContextFn create_root_context_for_testing) { - if (!base_wasm || !base_wasm->wasm()) { + if (!base_wasm) { if (!plugin->fail_open_) { ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::wasm), critical, "Plugin configured to fail closed failed to load"); } - // ThreadLocalObject cannot be nullptr, so create a WasmHandle holding nullptr. - return std::make_shared(nullptr); + return nullptr; } return std::static_pointer_cast(proxy_wasm::getOrCreateThreadLocalWasm( std::static_pointer_cast(base_wasm), plugin, diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index e47e68b394cd..d56dabe138c2 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -33,9 +33,9 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was FilterConfig::~FilterConfig() { if (tls_slot_->currentThreadRegistered()) { - tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { - if (handle.wasm()) { - handle.wasm()->startShutdown(plugin); + tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { + if (handle.has_value()) { + handle->wasm()->startShutdown(plugin); } }); } diff --git a/source/extensions/filters/http/wasm/wasm_filter.h b/source/extensions/filters/http/wasm/wasm_filter.h index bb37e240b8ac..67b90d079077 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.h +++ b/source/extensions/filters/http/wasm/wasm_filter.h @@ -26,14 +26,18 @@ class FilterConfig : Logger::Loggable { virtual ~FilterConfig(); std::shared_ptr createFilter() { - auto wasm = tls_slot_->get().wasm(); + Wasm* wasm = nullptr; + auto handle = tls_slot_->get(); + if (handle.has_value()) { + wasm = handle->wasm().get(); + } if (plugin_->fail_open_ && (!wasm || wasm->isFailed())) { return nullptr; } if (wasm && !root_context_id_) { root_context_id_ = wasm->getRootContext(plugin_, false)->id(); } - return std::make_shared(wasm.get(), root_context_id_, plugin_); + return std::make_shared(wasm, root_context_id_, plugin_); } private: diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index e4bbf17c35d7..8a9c153bf0b4 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -35,9 +35,9 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: FilterConfig::~FilterConfig() { if (tls_slot_->currentThreadRegistered()) { - tls_slot_->runOnAllThreads([plugin = plugin_](WasmHandle& handle) { - if (handle.wasm()) { - handle.wasm()->startShutdown(plugin); + tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { + if (handle.has_value()) { + handle->wasm()->startShutdown(plugin); } }); } diff --git a/source/extensions/filters/network/wasm/wasm_filter.h b/source/extensions/filters/network/wasm/wasm_filter.h index 58c57d59bc17..d720deef3fa6 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.h +++ b/source/extensions/filters/network/wasm/wasm_filter.h @@ -26,17 +26,21 @@ class FilterConfig : Logger::Loggable { ~FilterConfig(); std::shared_ptr createFilter() { - auto wasm = tls_slot_->get().wasm(); + Wasm* wasm = nullptr; + auto handle = tls_slot_->get(); + if (handle.has_value()) { + wasm = handle->wasm().get(); + } if (plugin_->fail_open_ && (!wasm || wasm->isFailed())) { return nullptr; } if (wasm && !root_context_id_) { root_context_id_ = wasm->getRootContext(plugin_, false)->id(); } - return std::make_shared(wasm.get(), root_context_id_, plugin_); + return std::make_shared(wasm, root_context_id_, plugin_); } - Envoy::Extensions::Common::Wasm::Wasm* wasmForTest() { return tls_slot_->get().wasm().get(); } + Envoy::Extensions::Common::Wasm::Wasm* wasmForTest() { return tls_slot_->get()->wasm().get(); } private: uint32_t root_context_id_{0}; From 487d9307048ea5de98c79449024ee770190b41d5 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Fri, 6 Nov 2020 01:57:42 +0000 Subject: [PATCH 11/15] review: add shutdown for a singleton Wasm Service. Signed-off-by: Piotr Sikora --- source/extensions/bootstrap/wasm/config.cc | 4 ++-- source/extensions/bootstrap/wasm/config.h | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index ed21ffdc3ea5..974654734b4c 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -37,8 +37,8 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con } if (singleton) { // Return a Wasm VM which will be stored as a singleton by the Server. - cb(std::make_unique( - Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, context.dispatcher()))); + cb(std::make_unique(plugin, Common::Wasm::getOrCreateThreadLocalWasm( + base_wasm, plugin, context.dispatcher()))); return; } // Per-thread WASM VM. diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 66b331546167..f7f8a11149e8 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -20,13 +20,16 @@ using Envoy::Extensions::Common::Wasm::WasmHandle; class WasmService { public: - WasmService(Common::Wasm::WasmHandleSharedPtr singleton) : singleton_(std::move(singleton)) {} + WasmService(Common::Wasm::PluginSharedPtr plugin, Common::Wasm::WasmHandleSharedPtr singleton) + : plugin_(plugin), singleton_(std::move(singleton)) {} WasmService(Common::Wasm::PluginSharedPtr plugin, ThreadLocal::TypedSlotPtr&& tls_slot) : plugin_(plugin), tls_slot_(std::move(tls_slot)) {} virtual ~WasmService() { - if (!singleton_ && tls_slot_->currentThreadRegistered()) { + if (singleton_) { + singleton_->wasm()->startShutdown(plugin_); + } else if (tls_slot_->currentThreadRegistered()) { tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { if (handle.has_value()) { handle->wasm()->startShutdown(plugin); From 930282dfdce0c03b09121f071011b02827017bcf Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Fri, 6 Nov 2020 01:58:38 +0000 Subject: [PATCH 12/15] review: add comments. Signed-off-by: Piotr Sikora --- source/extensions/access_loggers/wasm/wasm_access_log_impl.h | 1 + source/extensions/bootstrap/wasm/config.h | 2 ++ source/extensions/filters/http/wasm/wasm_filter.cc | 1 + source/extensions/filters/network/wasm/wasm_filter.cc | 1 + 4 files changed, 5 insertions(+) diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index f1690cfc28ed..329e6dbf8dff 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -46,6 +46,7 @@ class WasmAccessLog : public AccessLog::Instance { ~WasmAccessLog() override { if (tls_slot_->currentThreadRegistered()) { + // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { if (handle.has_value()) { handle->wasm()->startShutdown(plugin); diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index f7f8a11149e8..15e8463a3d61 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -28,8 +28,10 @@ class WasmService { virtual ~WasmService() { if (singleton_) { + // Singleton Wasm Service is running only on the main thread. singleton_->wasm()->startShutdown(plugin_); } else if (tls_slot_->currentThreadRegistered()) { + // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { if (handle.has_value()) { handle->wasm()->startShutdown(plugin); diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index d56dabe138c2..319e0e6b2792 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -33,6 +33,7 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was FilterConfig::~FilterConfig() { if (tls_slot_->currentThreadRegistered()) { + // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { if (handle.has_value()) { handle->wasm()->startShutdown(plugin); diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index 8a9c153bf0b4..ea9a1317dd3b 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -35,6 +35,7 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: FilterConfig::~FilterConfig() { if (tls_slot_->currentThreadRegistered()) { + // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { if (handle.has_value()) { handle->wasm()->startShutdown(plugin); From 9965dcdb70decce6813515dd788033f703651664 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Mon, 9 Nov 2020 23:24:05 +0000 Subject: [PATCH 13/15] review: use PluginHandle for shutdown-on-destroy semantics. Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 6 ++-- .../extensions/access_loggers/wasm/config.cc | 5 +-- .../wasm/wasm_access_log_impl.h | 21 +++--------- source/extensions/bootstrap/wasm/config.cc | 7 ++-- source/extensions/bootstrap/wasm/config.h | 29 +++++----------- source/extensions/common/wasm/context.h | 2 ++ source/extensions/common/wasm/wasm.cc | 21 ++++++++---- source/extensions/common/wasm/wasm.h | 25 +++++++++++--- .../extensions/common/wasm/wasm_extension.cc | 8 +++++ .../extensions/common/wasm/wasm_extension.h | 4 +++ .../filters/http/wasm/wasm_filter.cc | 16 ++------- .../filters/http/wasm/wasm_filter.h | 8 ++--- .../filters/network/wasm/wasm_filter.cc | 18 ++-------- .../filters/network/wasm/wasm_filter.h | 10 +++--- source/extensions/stat_sinks/wasm/config.cc | 2 +- .../stat_sinks/wasm/wasm_stat_sink_impl.h | 14 ++++---- test/extensions/common/wasm/wasm_test.cc | 34 ++++++++++++++----- .../filters/http/wasm/wasm_filter_test.cc | 6 +++- test/test_common/wasm_base.h | 4 ++- 19 files changed, 129 insertions(+), 111 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 7d939671e47f..97d104ce3173 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -830,10 +830,10 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "f08baacadbe656674414f0878e18852bed67b795", - sha256 = "dc3239c674d19198f71becf5d1c2bf159128af3c72ec2f9c532b3153ecb6d5a1", + version = "8c1403944be8c15abd04fd118366813fb9afba44", + sha256 = "179ff2b3936f0cdbadd06dc4e5b9f9f191c2b82d676f6424b95207e9c5cebe6c", strip_prefix = "proxy-wasm-cpp-host-{version}", - urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], + urls = ["https://github.com/PiotrSikora/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], extensions = [ "envoy.access_loggers.wasm", diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc index a0472415fed2..8ca765442e9c 100644 --- a/source/extensions/access_loggers/wasm/config.cc +++ b/source/extensions/access_loggers/wasm/config.cc @@ -37,9 +37,10 @@ WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_con auto callback = [access_log, &context, plugin](Common::Wasm::WasmHandleSharedPtr base_wasm) { // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. - auto tls_slot = ThreadLocal::TypedSlot::makeUnique(context.threadLocal()); + auto tls_slot = + ThreadLocal::TypedSlot::makeUnique(context.threadLocal()); tls_slot->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { - return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); + return Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, dispatcher); }); access_log->setTlsSlot(std::move(tls_slot)); }; diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index 329e6dbf8dff..94910ef56712 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -12,12 +12,12 @@ namespace Extensions { namespace AccessLoggers { namespace Wasm { +using Envoy::Extensions::Common::Wasm::PluginHandle; using Envoy::Extensions::Common::Wasm::PluginSharedPtr; -using Envoy::Extensions::Common::Wasm::WasmHandle; class WasmAccessLog : public AccessLog::Instance { public: - WasmAccessLog(const PluginSharedPtr& plugin, ThreadLocal::TypedSlotPtr&& tls_slot, + WasmAccessLog(const PluginSharedPtr& plugin, ThreadLocal::TypedSlotPtr&& tls_slot, AccessLog::FilterPtr filter) : plugin_(plugin), tls_slot_(std::move(tls_slot)), filter_(std::move(filter)) {} @@ -39,25 +39,14 @@ class WasmAccessLog : public AccessLog::Instance { } } - void setTlsSlot(ThreadLocal::TypedSlotPtr&& tls_slot) { + void setTlsSlot(ThreadLocal::TypedSlotPtr&& tls_slot) { ASSERT(tls_slot_ == nullptr); tls_slot_ = std::move(tls_slot); } - ~WasmAccessLog() override { - if (tls_slot_->currentThreadRegistered()) { - // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. - tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { - if (handle.has_value()) { - handle->wasm()->startShutdown(plugin); - } - }); - } - } - private: - Common::Wasm::PluginSharedPtr plugin_; - ThreadLocal::TypedSlotPtr tls_slot_; + PluginSharedPtr plugin_; + ThreadLocal::TypedSlotPtr tls_slot_; AccessLog::FilterPtr filter_; }; diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 974654734b4c..0e8f4caa99ac 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -37,15 +37,16 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con } if (singleton) { // Return a Wasm VM which will be stored as a singleton by the Server. - cb(std::make_unique(plugin, Common::Wasm::getOrCreateThreadLocalWasm( + cb(std::make_unique(plugin, Common::Wasm::getOrCreateThreadLocalPlugin( base_wasm, plugin, context.dispatcher()))); return; } // Per-thread WASM VM. // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. - auto tls_slot = ThreadLocal::TypedSlot::makeUnique(context.threadLocal()); + auto tls_slot = + ThreadLocal::TypedSlot::makeUnique(context.threadLocal()); tls_slot->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { - return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); + return Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, dispatcher); }); cb(std::make_unique(plugin, std::move(tls_slot))); }; diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 15e8463a3d61..b8f3850ef621 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -16,34 +16,21 @@ namespace Extensions { namespace Bootstrap { namespace Wasm { -using Envoy::Extensions::Common::Wasm::WasmHandle; +using Envoy::Extensions::Common::Wasm::PluginHandle; +using Envoy::Extensions::Common::Wasm::PluginHandleSharedPtr; +using Envoy::Extensions::Common::Wasm::PluginSharedPtr; class WasmService { public: - WasmService(Common::Wasm::PluginSharedPtr plugin, Common::Wasm::WasmHandleSharedPtr singleton) + WasmService(PluginSharedPtr plugin, PluginHandleSharedPtr singleton) : plugin_(plugin), singleton_(std::move(singleton)) {} - WasmService(Common::Wasm::PluginSharedPtr plugin, - ThreadLocal::TypedSlotPtr&& tls_slot) + WasmService(PluginSharedPtr plugin, ThreadLocal::TypedSlotPtr&& tls_slot) : plugin_(plugin), tls_slot_(std::move(tls_slot)) {} - virtual ~WasmService() { - if (singleton_) { - // Singleton Wasm Service is running only on the main thread. - singleton_->wasm()->startShutdown(plugin_); - } else if (tls_slot_->currentThreadRegistered()) { - // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. - tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { - if (handle.has_value()) { - handle->wasm()->startShutdown(plugin); - } - }); - } - } - private: - Common::Wasm::PluginSharedPtr plugin_; - Common::Wasm::WasmHandleSharedPtr singleton_; - ThreadLocal::TypedSlotPtr tls_slot_; + PluginSharedPtr plugin_; + PluginHandleSharedPtr singleton_; + ThreadLocal::TypedSlotPtr tls_slot_; }; using WasmServicePtr = std::unique_ptr; diff --git a/source/extensions/common/wasm/context.h b/source/extensions/common/wasm/context.h index e288c1e50602..657a0331addd 100644 --- a/source/extensions/common/wasm/context.h +++ b/source/extensions/common/wasm/context.h @@ -31,6 +31,7 @@ using proxy_wasm::ContextBase; using proxy_wasm::Pairs; using proxy_wasm::PairsWithStringValues; using proxy_wasm::PluginBase; +using proxy_wasm::PluginHandleBase; using proxy_wasm::SharedQueueDequeueToken; using proxy_wasm::SharedQueueEnqueueToken; using proxy_wasm::WasmBase; @@ -45,6 +46,7 @@ using GrpcService = envoy::config::core::v3::GrpcService; class Wasm; +using PluginHandleBaseSharedPtr = std::shared_ptr; using WasmHandleBaseSharedPtr = std::shared_ptr; // Opaque context object. diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index 6c939c989e86..1b0e8e513be2 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -281,6 +281,14 @@ getCloneFactory(WasmExtension* wasm_extension, Event::Dispatcher& dispatcher, }; } +static proxy_wasm::PluginHandleFactory getPluginFactory(WasmExtension* wasm_extension) { + auto wasm_plugin_factory = wasm_extension->pluginFactory(); + return [wasm_plugin_factory](WasmHandleBaseSharedPtr base_wasm, + absl::string_view plugin_key) -> std::shared_ptr { + return wasm_plugin_factory(std::static_pointer_cast(base_wasm), plugin_key); + }; +} + WasmEvent toWasmEvent(const std::shared_ptr& wasm) { if (!wasm) { return WasmEvent::UnableToCreateVM; @@ -474,10 +482,10 @@ bool createWasm(const VmConfig& vm_config, const PluginSharedPtr& plugin, create_root_context_for_testing); } -WasmHandleSharedPtr getOrCreateThreadLocalWasm(const WasmHandleSharedPtr& base_wasm, - const PluginSharedPtr& plugin, - Event::Dispatcher& dispatcher, - CreateContextFn create_root_context_for_testing) { +PluginHandleSharedPtr +getOrCreateThreadLocalPlugin(const WasmHandleSharedPtr& base_wasm, const PluginSharedPtr& plugin, + Event::Dispatcher& dispatcher, + CreateContextFn create_root_context_for_testing) { if (!base_wasm) { if (!plugin->fail_open_) { ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::wasm), critical, @@ -485,9 +493,10 @@ WasmHandleSharedPtr getOrCreateThreadLocalWasm(const WasmHandleSharedPtr& base_w } return nullptr; } - return std::static_pointer_cast(proxy_wasm::getOrCreateThreadLocalWasm( + return std::static_pointer_cast(proxy_wasm::getOrCreateThreadLocalPlugin( std::static_pointer_cast(base_wasm), plugin, - getCloneFactory(getWasmExtension(), dispatcher, create_root_context_for_testing))); + getCloneFactory(getWasmExtension(), dispatcher, create_root_context_for_testing), + getPluginFactory(getWasmExtension()))); } } // namespace Wasm diff --git a/source/extensions/common/wasm/wasm.h b/source/extensions/common/wasm/wasm.h index 882b7c8642b3..37091ff9a523 100644 --- a/source/extensions/common/wasm/wasm.h +++ b/source/extensions/common/wasm/wasm.h @@ -137,6 +137,23 @@ class WasmHandle : public WasmHandleBase, public ThreadLocal::ThreadLocalObject WasmSharedPtr wasm_; }; +using WasmHandleSharedPtr = std::shared_ptr; + +class PluginHandle : public PluginHandleBase, public ThreadLocal::ThreadLocalObject { +public: + explicit PluginHandle(const WasmHandleSharedPtr& wasm_handle, absl::string_view plugin_key) + : PluginHandleBase(std::static_pointer_cast(wasm_handle), plugin_key), + wasm_handle_(wasm_handle) {} + + WasmSharedPtr& wasm() { return wasm_handle_->wasm(); } + WasmHandleSharedPtr& wasmHandleForTest() { return wasm_handle_; } + +private: + WasmHandleSharedPtr wasm_handle_; +}; + +using PluginHandleSharedPtr = std::shared_ptr; + using CreateWasmCallback = std::function; // Returns false if createWasm failed synchronously. This is necessary because xDS *MUST* report @@ -151,10 +168,10 @@ bool createWasm(const VmConfig& vm_config, const PluginSharedPtr& plugin, CreateWasmCallback&& callback, CreateContextFn create_root_context_for_testing = nullptr); -WasmHandleSharedPtr -getOrCreateThreadLocalWasm(const WasmHandleSharedPtr& base_wasm, const PluginSharedPtr& plugin, - Event::Dispatcher& dispatcher, - CreateContextFn create_root_context_for_testing = nullptr); +PluginHandleSharedPtr +getOrCreateThreadLocalPlugin(const WasmHandleSharedPtr& base_wasm, const PluginSharedPtr& plugin, + Event::Dispatcher& dispatcher, + CreateContextFn create_root_context_for_testing = nullptr); void clearCodeCacheForTesting(); std::string anyToBytes(const ProtobufWkt::Any& any); diff --git a/source/extensions/common/wasm/wasm_extension.cc b/source/extensions/common/wasm/wasm_extension.cc index c75168f1761c..1917fa792a82 100644 --- a/source/extensions/common/wasm/wasm_extension.cc +++ b/source/extensions/common/wasm/wasm_extension.cc @@ -46,6 +46,14 @@ EnvoyWasm::createEnvoyWasmVmIntegration(const Stats::ScopeSharedPtr& scope, return std::make_unique(scope, runtime, short_runtime); } +PluginHandleExtensionFactory EnvoyWasm::pluginFactory() { + return [](const WasmHandleSharedPtr& base_wasm, + absl::string_view plugin_key) -> PluginHandleBaseSharedPtr { + return std::static_pointer_cast( + std::make_shared(base_wasm, plugin_key)); + }; +} + WasmHandleExtensionFactory EnvoyWasm::wasmFactory() { return [](const VmConfig vm_config, const Stats::ScopeSharedPtr& scope, Upstream::ClusterManager& cluster_manager, Event::Dispatcher& dispatcher, diff --git a/source/extensions/common/wasm/wasm_extension.h b/source/extensions/common/wasm/wasm_extension.h index 5d41a58bb337..22ae373162f2 100644 --- a/source/extensions/common/wasm/wasm_extension.h +++ b/source/extensions/common/wasm/wasm_extension.h @@ -31,6 +31,8 @@ class EnvoyWasmVmIntegration; using WasmHandleSharedPtr = std::shared_ptr; using CreateContextFn = std::function& plugin)>; +using PluginHandleExtensionFactory = std::function; using WasmHandleExtensionFactory = std::function { virtual std::unique_ptr createEnvoyWasmVmIntegration(const Stats::ScopeSharedPtr& scope, absl::string_view runtime, absl::string_view short_runtime) = 0; + virtual PluginHandleExtensionFactory pluginFactory() = 0; virtual WasmHandleExtensionFactory wasmFactory() = 0; virtual WasmHandleExtensionCloneFactory wasmCloneFactory() = 0; enum class WasmEvent : int { @@ -100,6 +103,7 @@ class EnvoyWasm : public WasmExtension { std::unique_ptr createEnvoyWasmVmIntegration(const Stats::ScopeSharedPtr& scope, absl::string_view runtime, absl::string_view short_runtime) override; + PluginHandleExtensionFactory pluginFactory() override; WasmHandleExtensionFactory wasmFactory() override; WasmHandleExtensionCloneFactory wasmCloneFactory() override; void onEvent(WasmEvent event, const PluginSharedPtr& plugin) override; diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc index 319e0e6b2792..90713ba01989 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.cc +++ b/source/extensions/filters/http/wasm/wasm_filter.cc @@ -7,7 +7,8 @@ namespace Wasm { FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& config, Server::Configuration::FactoryContext& context) - : tls_slot_(ThreadLocal::TypedSlot::makeUnique(context.threadLocal())) { + : tls_slot_( + ThreadLocal::TypedSlot::makeUnique(context.threadLocal())) { plugin_ = std::make_shared( config.config().name(), config.config().root_id(), config.config().vm_config().vm_id(), config.config().vm_config().runtime(), @@ -18,7 +19,7 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was auto callback = [plugin, this](const Common::Wasm::WasmHandleSharedPtr& base_wasm) { // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. tls_slot_->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { - return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); + return Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, dispatcher); }); }; @@ -31,17 +32,6 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was } } -FilterConfig::~FilterConfig() { - if (tls_slot_->currentThreadRegistered()) { - // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. - tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { - if (handle.has_value()) { - handle->wasm()->startShutdown(plugin); - } - }); - } -} - } // namespace Wasm } // namespace HttpFilters } // namespace Extensions diff --git a/source/extensions/filters/http/wasm/wasm_filter.h b/source/extensions/filters/http/wasm/wasm_filter.h index 67b90d079077..956862e5f09b 100644 --- a/source/extensions/filters/http/wasm/wasm_filter.h +++ b/source/extensions/filters/http/wasm/wasm_filter.h @@ -16,14 +16,14 @@ namespace HttpFilters { namespace Wasm { using Envoy::Extensions::Common::Wasm::Context; +using Envoy::Extensions::Common::Wasm::PluginHandle; +using Envoy::Extensions::Common::Wasm::PluginSharedPtr; using Envoy::Extensions::Common::Wasm::Wasm; -using Envoy::Extensions::Common::Wasm::WasmHandle; class FilterConfig : Logger::Loggable { public: FilterConfig(const envoy::extensions::filters::http::wasm::v3::Wasm& proto_config, Server::Configuration::FactoryContext& context); - virtual ~FilterConfig(); std::shared_ptr createFilter() { Wasm* wasm = nullptr; @@ -42,8 +42,8 @@ class FilterConfig : Logger::Loggable { private: uint32_t root_context_id_{0}; - Envoy::Extensions::Common::Wasm::PluginSharedPtr plugin_; - ThreadLocal::TypedSlotPtr tls_slot_; + PluginSharedPtr plugin_; + ThreadLocal::TypedSlotPtr tls_slot_; Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_; }; diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc index ea9a1317dd3b..ccceeb9dc478 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.cc +++ b/source/extensions/filters/network/wasm/wasm_filter.cc @@ -5,11 +5,10 @@ namespace Extensions { namespace NetworkFilters { namespace Wasm { -using Envoy::Extensions::Common::Wasm::WasmHandle; - FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3::Wasm& config, Server::Configuration::FactoryContext& context) - : tls_slot_(ThreadLocal::TypedSlot::makeUnique(context.threadLocal())) { + : tls_slot_( + ThreadLocal::TypedSlot::makeUnique(context.threadLocal())) { plugin_ = std::make_shared( config.config().name(), config.config().root_id(), config.config().vm_config().vm_id(), config.config().vm_config().runtime(), @@ -20,7 +19,7 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: auto callback = [plugin, this](Common::Wasm::WasmHandleSharedPtr base_wasm) { // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call. tls_slot_->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { - return Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, dispatcher); + return Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, dispatcher); }); }; @@ -33,17 +32,6 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3:: } } -FilterConfig::~FilterConfig() { - if (tls_slot_->currentThreadRegistered()) { - // Start graceful shutdown of Wasm plugin, unless Envoy is already shutting down. - tls_slot_->runOnAllThreads([plugin = plugin_](OptRef handle) { - if (handle.has_value()) { - handle->wasm()->startShutdown(plugin); - } - }); - } -} - } // namespace Wasm } // namespace NetworkFilters } // namespace Extensions diff --git a/source/extensions/filters/network/wasm/wasm_filter.h b/source/extensions/filters/network/wasm/wasm_filter.h index d720deef3fa6..6a6fe2584b2c 100644 --- a/source/extensions/filters/network/wasm/wasm_filter.h +++ b/source/extensions/filters/network/wasm/wasm_filter.h @@ -16,14 +16,14 @@ namespace NetworkFilters { namespace Wasm { using Envoy::Extensions::Common::Wasm::Context; +using Envoy::Extensions::Common::Wasm::PluginHandle; +using Envoy::Extensions::Common::Wasm::PluginSharedPtr; using Envoy::Extensions::Common::Wasm::Wasm; -using Envoy::Extensions::Common::Wasm::WasmHandle; class FilterConfig : Logger::Loggable { public: FilterConfig(const envoy::extensions::filters::network::wasm::v3::Wasm& proto_config, Server::Configuration::FactoryContext& context); - ~FilterConfig(); std::shared_ptr createFilter() { Wasm* wasm = nullptr; @@ -40,12 +40,12 @@ class FilterConfig : Logger::Loggable { return std::make_shared(wasm, root_context_id_, plugin_); } - Envoy::Extensions::Common::Wasm::Wasm* wasmForTest() { return tls_slot_->get()->wasm().get(); } + Wasm* wasmForTest() { return tls_slot_->get()->wasm().get(); } private: uint32_t root_context_id_{0}; - Envoy::Extensions::Common::Wasm::PluginSharedPtr plugin_; - ThreadLocal::TypedSlotPtr tls_slot_; + PluginSharedPtr plugin_; + ThreadLocal::TypedSlotPtr tls_slot_; Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_; }; diff --git a/source/extensions/stat_sinks/wasm/config.cc b/source/extensions/stat_sinks/wasm/config.cc index 44233a56374f..da07bbdd5880 100644 --- a/source/extensions/stat_sinks/wasm/config.cc +++ b/source/extensions/stat_sinks/wasm/config.cc @@ -40,7 +40,7 @@ WasmSinkFactory::createStatsSink(const Protobuf::Message& proto_config, return; } wasm_sink->setSingleton( - Common::Wasm::getOrCreateThreadLocalWasm(base_wasm, plugin, context.dispatcher())); + Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, context.dispatcher())); }; if (!Common::Wasm::createWasm( diff --git a/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h b/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h index 0293ab5d5efc..5b339a6c80e9 100644 --- a/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h +++ b/source/extensions/stat_sinks/wasm/wasm_stat_sink_impl.h @@ -9,21 +9,21 @@ namespace Extensions { namespace StatSinks { namespace Wasm { +using Envoy::Extensions::Common::Wasm::PluginHandleSharedPtr; using Envoy::Extensions::Common::Wasm::PluginSharedPtr; -using Envoy::Extensions::Common::Wasm::WasmHandle; class WasmStatSink : public Stats::Sink { public: - WasmStatSink(const PluginSharedPtr& plugin, Common::Wasm::WasmHandleSharedPtr singleton) - : plugin_(plugin), singleton_(std::move(singleton)) {} + WasmStatSink(const PluginSharedPtr& plugin, PluginHandleSharedPtr singleton) + : plugin_(plugin), singleton_(singleton) {} void flush(Stats::MetricSnapshot& snapshot) override { singleton_->wasm()->onStatsUpdate(plugin_, snapshot); } - void setSingleton(Common::Wasm::WasmHandleSharedPtr singleton) { + void setSingleton(PluginHandleSharedPtr singleton) { ASSERT(singleton != nullptr); - singleton_ = std::move(singleton); + singleton_ = singleton; } void onHistogramComplete(const Stats::Histogram& histogram, uint64_t value) override { @@ -32,8 +32,8 @@ class WasmStatSink : public Stats::Sink { } private: - Common::Wasm::PluginSharedPtr plugin_; - Common::Wasm::WasmHandleSharedPtr singleton_; + PluginSharedPtr plugin_; + PluginHandleSharedPtr singleton_; }; } // namespace Wasm diff --git a/test/extensions/common/wasm/wasm_test.cc b/test/extensions/common/wasm/wasm_test.cc index 4d656f112aab..46c2c8937132 100644 --- a/test/extensions/common/wasm/wasm_test.cc +++ b/test/extensions/common/wasm/wasm_test.cc @@ -689,7 +689,7 @@ TEST_P(WasmCommonTest, VmCache) { EXPECT_NE(wasm_handle2, nullptr); EXPECT_EQ(wasm_handle, wasm_handle2); - auto wasm_handle_local = getOrCreateThreadLocalWasm( + auto plugin_handle_local = getOrCreateThreadLocalPlugin( wasm_handle, plugin, [&dispatcher](const WasmHandleBaseSharedPtr& base_wasm) -> WasmHandleBaseSharedPtr { auto wasm = @@ -703,12 +703,17 @@ TEST_P(WasmCommonTest, VmCache) { return root_context; }); return std::make_shared(wasm); + }, + [](const WasmHandleBaseSharedPtr& base_wasm, + absl::string_view plugin_key) -> PluginHandleBaseSharedPtr { + return std::make_shared(std::static_pointer_cast(base_wasm), + plugin_key); }); wasm_handle.reset(); wasm_handle2.reset(); - auto wasm = wasm_handle_local->wasm(); - wasm_handle_local.reset(); + auto wasm = plugin_handle_local->wasm(); + plugin_handle_local.reset(); dispatcher->run(Event::Dispatcher::RunType::NonBlock); wasm->configure(wasm->getContext(1), plugin); @@ -789,7 +794,7 @@ TEST_P(WasmCommonTest, RemoteCode) { EXPECT_NE(wasm_handle, nullptr); - auto wasm_handle_local = getOrCreateThreadLocalWasm( + auto plugin_handle_local = getOrCreateThreadLocalPlugin( wasm_handle, plugin, [&dispatcher](const WasmHandleBaseSharedPtr& base_wasm) -> WasmHandleBaseSharedPtr { auto wasm = @@ -803,11 +808,17 @@ TEST_P(WasmCommonTest, RemoteCode) { return root_context; }); return std::make_shared(wasm); + }, + [](const WasmHandleBaseSharedPtr& base_wasm, + absl::string_view plugin_key) -> PluginHandleBaseSharedPtr { + return std::make_shared(std::static_pointer_cast(base_wasm), + plugin_key); }); wasm_handle.reset(); - auto wasm = wasm_handle_local->wasm(); - wasm_handle_local.reset(); + auto wasm = plugin_handle_local->wasm(); + plugin_handle_local.reset(); + dispatcher->run(Event::Dispatcher::RunType::NonBlock); wasm->configure(wasm->getContext(1), plugin); plugin.reset(); @@ -899,7 +910,7 @@ TEST_P(WasmCommonTest, RemoteCodeMultipleRetry) { dispatcher->run(Event::Dispatcher::RunType::NonBlock); EXPECT_NE(wasm_handle, nullptr); - auto wasm_handle_local = getOrCreateThreadLocalWasm( + auto plugin_handle_local = getOrCreateThreadLocalPlugin( wasm_handle, plugin, [&dispatcher](const WasmHandleBaseSharedPtr& base_wasm) -> WasmHandleBaseSharedPtr { auto wasm = @@ -913,11 +924,16 @@ TEST_P(WasmCommonTest, RemoteCodeMultipleRetry) { return root_context; }); return std::make_shared(wasm); + }, + [](const WasmHandleBaseSharedPtr& base_wasm, + absl::string_view plugin_key) -> PluginHandleBaseSharedPtr { + return std::make_shared(std::static_pointer_cast(base_wasm), + plugin_key); }); wasm_handle.reset(); - auto wasm = wasm_handle_local->wasm(); - wasm_handle_local.reset(); + auto wasm = plugin_handle_local->wasm(); + plugin_handle_local.reset(); dispatcher->run(Event::Dispatcher::RunType::NonBlock); wasm->configure(wasm->getContext(1), plugin); diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc index ba6819f1d8b3..f0a0a7253397 100644 --- a/test/extensions/filters/http/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc @@ -716,6 +716,7 @@ TEST_P(WasmHttpFilterTest, AsyncCallAfterDestroyed) { // Destroy the Context, Plugin and VM. context_.reset(); plugin_.reset(); + plugin_handle_.reset(); wasm_.reset(); Http::ResponseMessagePtr response_message(new Http::ResponseMessageImpl( @@ -1010,6 +1011,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallAfterDestroyed) { // Destroy the Context, Plugin and VM. context_.reset(); plugin_.reset(); + plugin_handle_.reset(); wasm_.reset(); ProtobufWkt::Value value; @@ -1203,6 +1205,7 @@ TEST_P(WasmHttpFilterTest, GrpcStreamOpenAtShutdown) { // Destroy the Context, Plugin and VM. context_.reset(); plugin_.reset(); + plugin_handle_.reset(); wasm_.reset(); } @@ -1436,7 +1439,8 @@ TEST_P(WasmHttpFilterTest, RootId2) { setupFilter(); EXPECT_CALL(filter(), log_(spdlog::level::debug, Eq(absl::string_view("onRequestHeaders2 2")))); Http::TestRequestHeaderMapImpl request_headers; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, true)); + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, + filter().decodeHeaders(request_headers, true)); } } // namespace Wasm diff --git a/test/test_common/wasm_base.h b/test/test_common/wasm_base.h index a8231c0c1ca2..2cfc796084eb 100644 --- a/test/test_common/wasm_base.h +++ b/test/test_common/wasm_base.h @@ -80,12 +80,13 @@ template class WasmTestBase : public Base { lifecycle_notifier_, remote_data_provider_, [this](WasmHandleSharedPtr wasm) { wasm_ = wasm; }, create_root); if (wasm_) { - wasm_ = getOrCreateThreadLocalWasm( + plugin_handle_ = getOrCreateThreadLocalPlugin( wasm_, plugin_, dispatcher_, [this, create_root](Wasm* wasm, const std::shared_ptr& plugin) { root_context_ = static_cast(create_root(wasm, plugin)); return root_context_; }); + wasm_ = plugin_handle_->wasmHandleForTest(); } } @@ -101,6 +102,7 @@ template class WasmTestBase : public Base { NiceMock init_manager_; WasmHandleSharedPtr wasm_; PluginSharedPtr plugin_; + PluginHandleSharedPtr plugin_handle_; NiceMock ssl_; NiceMock connection_; NiceMock decoder_callbacks_; From 3d4e28e933e27b0a51ca4f45a927324aaac60a73 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Tue, 10 Nov 2020 09:10:34 +0000 Subject: [PATCH 14/15] review: point at merged commit. Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 97d104ce3173..3ffbe078a473 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -830,10 +830,10 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "8c1403944be8c15abd04fd118366813fb9afba44", - sha256 = "179ff2b3936f0cdbadd06dc4e5b9f9f191c2b82d676f6424b95207e9c5cebe6c", + version = "eceb02d5b7772ec1cd78a4d35356e57d2e6d59bb", + sha256 = "ae9d9b87d21d95647ebda197d130b37bddc5c6ee3e6630909a231fd55fcc9069", strip_prefix = "proxy-wasm-cpp-host-{version}", - urls = ["https://github.com/PiotrSikora/proxy-wasm-cpp-host/archive/{version}.tar.gz"], + urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], extensions = [ "envoy.access_loggers.wasm", From d128aaf1fcb08349431b10bfc240e9bfd3fab83b Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Tue, 10 Nov 2020 09:15:14 +0000 Subject: [PATCH 15/15] review: update release date. Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 3ffbe078a473..74980c6c205d 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -842,7 +842,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - release_date = "2020-10-30", + release_date = "2020-11-10", cpe = "N/A", ), emscripten_toolchain = dict(