From 97a8d1fda175b4cfd742e1838debef67f4dbbb53 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Thu, 19 Aug 2021 17:28:04 +0900 Subject: [PATCH 1/3] refactor: decouple foreign functions from WasmBase. Signed-off-by: Takeshi Yoneda --- include/proxy-wasm/exports.h | 30 ++++++++++++++++++++++++++++++ include/proxy-wasm/wasm.h | 9 --------- src/exports.cc | 20 +++++++++++++++++++- src/wasm.cc | 16 ---------------- 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/include/proxy-wasm/exports.h b/include/proxy-wasm/exports.h index ded6419a4..02a4438ef 100644 --- a/include/proxy-wasm/exports.h +++ b/include/proxy-wasm/exports.h @@ -30,6 +30,36 @@ ::proxy_wasm::ContextBase *contextOrEffectiveContext(); extern thread_local ContextBase *current_context_; +/** + * WasmForeignFunction is used for registering host-specific host functions. + * A foreign function can be registered via RegisterForeignFunction and available + * to Wasm modules via proxy_call_foreign_function. + * @param wasm is the WasmBase which the Wasm module is running on. + * @param argument is the view to the argument to the function passed by the module. + * @param alloc_result is used to allocate the result data of this foreign function. + */ +using WasmForeignFunction = std::function alloc_result)>; + +/** + * Used to get the foreign function registered via RegisterForeignFunction for a given name. + * @param function_name is the name used to lookup the foreign function table. + * @return a WasmForeignFunction if registered. + */ +WasmForeignFunction getForeignFunction(std::string_view function_name); + +/** + * RegisterForeignFunction is used to register a foreign function in the lookup table + * used internally in getForeignFunction. + */ +struct RegisterForeignFunction { + /** + * @param function_name is the key for this foreign function. + * @param f is the function instance. + */ + RegisterForeignFunction(std::string function_name, WasmForeignFunction f); +}; + namespace exports { template size_t pairsSize(const Pairs &result) { diff --git a/include/proxy-wasm/wasm.h b/include/proxy-wasm/wasm.h index 78ffbe370..f35e6694c 100644 --- a/include/proxy-wasm/wasm.h +++ b/include/proxy-wasm/wasm.h @@ -34,11 +34,8 @@ namespace proxy_wasm { #include "proxy_wasm_common.h" class ContextBase; -class WasmBase; class WasmHandleBase; -using WasmForeignFunction = - std::function)>; using WasmVmFactory = std::function()>; using CallOnThreadFunction = std::function)>; @@ -129,8 +126,6 @@ class WasmBase : public std::enable_shared_from_this { bool copyToPointerSize(std::string_view s, uint64_t ptr_ptr, uint64_t size_ptr); template bool setDatatype(uint64_t ptr, const T &t); - WasmForeignFunction getForeignFunction(std::string_view function_name); - void fail(FailState fail_state, std::string_view message) { error(message); failed_ = fail_state; @@ -439,8 +434,4 @@ template inline bool WasmBase::setDatatype(uint64_t ptr, const T &t return wasm_vm_->setMemory(ptr, sizeof(T), &t); } -struct RegisterForeignFunction { - RegisterForeignFunction(std::string name, WasmForeignFunction f); -}; - } // namespace proxy_wasm diff --git a/src/exports.cc b/src/exports.cc index 0922b2df5..f773eff91 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -36,6 +36,24 @@ ContextBase *contextOrEffectiveContext() { // of current_context_. extern thread_local uint32_t effective_context_id_; +// Lookup table of foreign functions, using a pointer to avoid the initialization fiasco. +std::unordered_map *foreign_functions = nullptr; + +WasmForeignFunction getForeignFunction(std::string_view function_name) { + auto it = foreign_functions->find(std::string(function_name)); + if (it != foreign_functions->end()) { + return it->second; + } + return nullptr; +} + +RegisterForeignFunction::RegisterForeignFunction(std::string name, WasmForeignFunction f) { + if (!foreign_functions) { + foreign_functions = new std::remove_reference::type; + } + (*foreign_functions)[name] = f; +} + namespace exports { namespace { @@ -220,7 +238,7 @@ Word call_foreign_function(Word function_name, Word function_name_size, Word arg if (!args_opt) { return WasmResult::InvalidMemoryAccess; } - auto f = context->wasm()->getForeignFunction(function.value()); + auto f = getForeignFunction(function.value()); if (!f) { return WasmResult::NotFound; } diff --git a/src/wasm.cc b/src/wasm.cc index 4707c2141..87a53ef54 100644 --- a/src/wasm.cc +++ b/src/wasm.cc @@ -45,7 +45,6 @@ thread_local std::unordered_map> lo // Map from Wasm Key to the base Wasm instance, using a pointer to avoid the initialization fiasco. std::mutex base_wasms_mutex; std::unordered_map> *base_wasms = nullptr; -std::unordered_map *foreign_functions = nullptr; std::vector Sha256(const std::vector parts) { uint8_t sha256[SHA256_DIGEST_LENGTH]; @@ -85,13 +84,6 @@ class WasmBase::ShutdownHandle { std::shared_ptr wasm_; }; -RegisterForeignFunction::RegisterForeignFunction(std::string name, WasmForeignFunction f) { - if (!foreign_functions) { - foreign_functions = new std::remove_reference::type; - } - (*foreign_functions)[name] = f; -} - void WasmBase::registerCallbacks() { #define _REGISTER(_fn) \ wasm_vm_->registerCallback( \ @@ -454,14 +446,6 @@ void WasmBase::finishShutdown() { } } -WasmForeignFunction WasmBase::getForeignFunction(std::string_view function_name) { - auto it = foreign_functions->find(std::string(function_name)); - if (it != foreign_functions->end()) { - return it->second; - } - return nullptr; -} - std::shared_ptr createWasm(std::string vm_key, std::string code, std::shared_ptr plugin, WasmHandleFactory factory, From 7893fc75855a0057741b71917224860186037d45 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Wed, 25 Aug 2021 10:24:43 +0900 Subject: [PATCH 2/3] review: add guard both for registerer/getter. Signed-off-by: Takeshi Yoneda --- src/exports.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/exports.cc b/src/exports.cc index f773eff91..06e25c1a1 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -36,22 +36,22 @@ ContextBase *contextOrEffectiveContext() { // of current_context_. extern thread_local uint32_t effective_context_id_; -// Lookup table of foreign functions, using a pointer to avoid the initialization fiasco. -std::unordered_map *foreign_functions = nullptr; +std::unordered_map &foreignFunctions() { + static auto ptr = new std::unordered_map; + return *ptr; +} WasmForeignFunction getForeignFunction(std::string_view function_name) { - auto it = foreign_functions->find(std::string(function_name)); - if (it != foreign_functions->end()) { + auto foreign_functions = foreignFunctions(); + auto it = foreign_functions.find(std::string(function_name)); + if (it != foreign_functions.end()) { return it->second; } return nullptr; } RegisterForeignFunction::RegisterForeignFunction(std::string name, WasmForeignFunction f) { - if (!foreign_functions) { - foreign_functions = new std::remove_reference::type; - } - (*foreign_functions)[name] = f; + foreignFunctions()[name] = f; } namespace exports { From ab4b499532700849f1247104b7ae2f0ef24bff83 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Wed, 25 Aug 2021 10:24:57 +0900 Subject: [PATCH 3/3] fix style Signed-off-by: Takeshi Yoneda --- src/exports.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/exports.cc b/src/exports.cc index 06e25c1a1..f56343699 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -37,8 +37,8 @@ ContextBase *contextOrEffectiveContext() { extern thread_local uint32_t effective_context_id_; std::unordered_map &foreignFunctions() { - static auto ptr = new std::unordered_map; - return *ptr; + static auto ptr = new std::unordered_map; + return *ptr; } WasmForeignFunction getForeignFunction(std::string_view function_name) {