Merge branch 'proxy-wasm:master' into master

Author: rahulchaphalkar

.github/workflows/cpp.yml

@@ -35,6 +35,9 @@ jobs:
 
     steps:
     - uses: actions/checkout@v2
+    - uses: actions/setup-go@v2
+      with:
+        go-version: '^1.16'
 
     - name: Format (clang-format)
       run: |
@@ -44,13 +47,13 @@ jobs:
 
     - name: Format (buildifier)
       run: |
-        go get -u github.com/bazelbuild/buildtools/buildifier
+        go install github.com/bazelbuild/buildtools/buildifier@latest
         export PATH=$PATH:$(go env GOPATH)/bin
         find . -name "BUILD" | xargs -n1 buildifier -mode=check
 
     - name: Format (addlicense)
       run: |
-        go get -u github.com/google/addlicense
+        go install github.com/google/addlicense@latest
         export PATH=$PATH:$(go env GOPATH)/bin
         addlicense -check .
 

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<WasmResult(
+    WasmBase &wasm, std::string_view argument, std::function<void *(size_t size)> 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 <typename Pairs> size_t pairsSize(const Pairs &result) {

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<WasmResult(WasmBase &, std::string_view, std::function<void *(size_t size)>)>;
 using WasmVmFactory = std::function<std::unique_ptr<WasmVm>()>;
 using CallOnThreadFunction = std::function<void(std::function<void()>)>;
 
@@ -129,8 +126,6 @@ class WasmBase : public std::enable_shared_from_this<WasmBase> {
   bool copyToPointerSize(std::string_view s, uint64_t ptr_ptr, uint64_t size_ptr);
   template <typename T> 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;
@@ -176,6 +171,35 @@ class WasmBase : public std::enable_shared_from_this<WasmBase> {
   uint32_t nextGaugeMetricId() { return next_gauge_metric_id_ += kMetricIdIncrement; }
   uint32_t nextHistogramMetricId() { return next_histogram_metric_id_ += kMetricIdIncrement; }
 
+  enum class CalloutType : uint32_t {
+    HttpCall = 0,
+    GrpcCall = 1,
+    GrpcStream = 2,
+  };
+  static const uint32_t kCalloutTypeMask = 0x3;  // Enough to cover the 3 types.
+  static const uint32_t kCalloutIncrement = 0x4; // Enough to cover the 3 types.
+  bool isHttpCallId(uint32_t callout_id) {
+    return (callout_id & kCalloutTypeMask) == static_cast<uint32_t>(CalloutType::HttpCall);
+  }
+  bool isGrpcCallId(uint32_t callout_id) {
+    return (callout_id & kCalloutTypeMask) == static_cast<uint32_t>(CalloutType::GrpcCall);
+  }
+  bool isGrpcStreamId(uint32_t callout_id) {
+    return (callout_id & kCalloutTypeMask) == static_cast<uint32_t>(CalloutType::GrpcStream);
+  }
+  uint32_t nextHttpCallId() {
+    // TODO(PiotrSikora): re-add rollover protection (requires at least 1 billion callouts).
+    return next_http_call_id_ += kCalloutIncrement;
+  }
+  uint32_t nextGrpcCallId() {
+    // TODO(PiotrSikora): re-add rollover protection (requires at least 1 billion callouts).
+    return next_grpc_call_id_ += kCalloutIncrement;
+  }
+  uint32_t nextGrpcStreamId() {
+    // TODO(PiotrSikora): re-add rollover protection (requires at least 1 billion callouts).
+    return next_grpc_stream_id_ += kCalloutIncrement;
+  }
+
 protected:
   friend class ContextBase;
   class ShutdownHandle;
@@ -279,6 +303,11 @@ class WasmBase : public std::enable_shared_from_this<WasmBase> {
   uint32_t next_gauge_metric_id_ = static_cast<uint32_t>(MetricType::Gauge);
   uint32_t next_histogram_metric_id_ = static_cast<uint32_t>(MetricType::Histogram);
 
+  // HTTP/gRPC callouts.
+  uint32_t next_http_call_id_ = static_cast<uint32_t>(CalloutType::HttpCall);
+  uint32_t next_grpc_call_id_ = static_cast<uint32_t>(CalloutType::GrpcCall);
+  uint32_t next_grpc_stream_id_ = static_cast<uint32_t>(CalloutType::GrpcStream);
+
   // Actions to be done after the call into the VM returns.
   std::deque<std::function<void()>> after_vm_call_actions_;
 
@@ -405,8 +434,4 @@ template <typename T> 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

include/proxy-wasm/wasm_vm.h

@@ -20,6 +20,7 @@
 #include <optional>
 #include <string>
 #include <unordered_map>
+#include <vector>
 
 #include "include/proxy-wasm/word.h"
 
@@ -176,8 +177,8 @@ class WasmVm {
 public:
   virtual ~WasmVm() = default;
   /**
-   * Return the runtime identifier.
-   * @return one of WasmRuntimeValues from well_known_names.h (e.g. "v8").
+   * Identify the Wasm runtime.
+   * @return the name of the underlying Wasm runtime.
    */
   virtual std::string_view runtime() = 0;
 
@@ -297,12 +298,12 @@ class WasmVm {
   void fail(FailState fail_state, std::string_view message) {
     integration()->error(message);
     failed_ = fail_state;
-    if (fail_callback_) {
-      fail_callback_(fail_state);
+    for (auto &callback : fail_callbacks_) {
+      callback(fail_state);
     }
   }
-  void setFailCallback(std::function<void(FailState)> fail_callback) {
-    fail_callback_ = fail_callback;
+  void addFailCallback(std::function<void(FailState)> fail_callback) {
+    fail_callbacks_.push_back(fail_callback);
   }
 
   // Integrator operations.
@@ -312,7 +313,7 @@ class WasmVm {
 protected:
   std::unique_ptr<WasmVmIntegration> integration_;
   FailState failed_ = FailState::Ok;
-  std::function<void(FailState)> fail_callback_;
+  std::vector<std::function<void(FailState)>> fail_callbacks_;
 };
 
 // Thread local state set during a call into a WASM VM so that calls coming out of the

src/exports.cc

@@ -36,6 +36,24 @@ ContextBase *contextOrEffectiveContext() {
 // of current_context_.
 extern thread_local uint32_t effective_context_id_;
 
+std::unordered_map<std::string, WasmForeignFunction> &foreignFunctions() {
+  static auto ptr = new std::unordered_map<std::string, WasmForeignFunction>;
+  return *ptr;
+}
+
+WasmForeignFunction getForeignFunction(std::string_view function_name) {
+  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) {
+  foreignFunctions()[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;
   }

src/wasm.cc

@@ -45,7 +45,6 @@ thread_local std::unordered_map<std::string, std::weak_ptr<PluginHandleBase>> 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<std::string, std::weak_ptr<WasmHandleBase>> *base_wasms = nullptr;
-std::unordered_map<std::string, WasmForeignFunction> *foreign_functions = nullptr;
 
 std::vector<uint8_t> Sha256(const std::vector<std::string_view> parts) {
   uint8_t sha256[SHA256_DIGEST_LENGTH];
@@ -85,13 +84,6 @@ class WasmBase::ShutdownHandle {
   std::shared_ptr<WasmBase> wasm_;
 };
 
-RegisterForeignFunction::RegisterForeignFunction(std::string name, WasmForeignFunction f) {
-  if (!foreign_functions) {
-    foreign_functions = new std::remove_reference<decltype(*foreign_functions)>::type;
-  }
-  (*foreign_functions)[name] = f;
-}
-
 void WasmBase::registerCallbacks() {
 #define _REGISTER(_fn)                                                                             \
   wasm_vm_->registerCallback(                                                                      \
@@ -208,7 +200,7 @@ WasmBase::WasmBase(const std::shared_ptr<WasmHandleBase> &base_wasm_handle, Wasm
   if (!wasm_vm_) {
     failed_ = FailState::UnableToCreateVm;
   } else {
-    wasm_vm_->setFailCallback([this](FailState fail_state) { failed_ = fail_state; });
+    wasm_vm_->addFailCallback([this](FailState fail_state) { failed_ = fail_state; });
   }
 }
 
@@ -222,7 +214,7 @@ WasmBase::WasmBase(std::unique_ptr<WasmVm> wasm_vm, std::string_view vm_id,
   if (!wasm_vm_) {
     failed_ = FailState::UnableToCreateVm;
   } else {
-    wasm_vm_->setFailCallback([this](FailState fail_state) { failed_ = fail_state; });
+    wasm_vm_->addFailCallback([this](FailState fail_state) { failed_ = fail_state; });
   }
 }
 
@@ -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<WasmHandleBase> createWasm(std::string vm_key, std::string code,
                                            std::shared_ptr<PluginBase> plugin,
                                            WasmHandleFactory factory,
@@ -559,6 +543,14 @@ getOrCreateThreadLocalWasm(std::shared_ptr<WasmHandleBase> base_handle,
     return nullptr;
   }
   local_wasms[vm_key] = wasm_handle;
+  wasm_handle->wasm()->wasm_vm()->addFailCallback([vm_key](proxy_wasm::FailState fail_state) {
+    if (fail_state == proxy_wasm::FailState::RuntimeError) {
+      // If VM failed, erase the entry so that:
+      // 1) we can recreate the new thread local VM from the same base_wasm.
+      // 2) we wouldn't reuse the failed VM for new plugins accidentally.
+      local_wasms.erase(vm_key);
+    };
+  });
   return wasm_handle;
 }
 
@@ -594,6 +586,14 @@ std::shared_ptr<PluginHandleBase> getOrCreateThreadLocalPlugin(
   }
   auto plugin_handle = plugin_factory(wasm_handle, plugin);
   local_plugins[key] = plugin_handle;
+  wasm_handle->wasm()->wasm_vm()->addFailCallback([key](proxy_wasm::FailState fail_state) {
+    if (fail_state == proxy_wasm::FailState::RuntimeError) {
+      // If VM failed, erase the entry so that:
+      // 1) we can recreate the new thread local plugin from the same base_wasm.
+      // 2) we wouldn't reuse the failed VM for new plugin configs accidentally.
+      local_plugins.erase(key);
+    };
+  });
   return plugin_handle;
 }
 

test/BUILD

@@ -78,9 +78,13 @@ cc_test(
 )
 
 cc_test(
-    name = "context_test",
-    srcs = ["context_test.cc"],
+    name = "wasm_test",
+    srcs = ["wasm_test.cc"],
+    data = [
+        "//test/test_data:abi_export.wasm",
+    ],
     deps = [
+        ":utility_lib",
         "//:lib",
         "@com_google_googletest//:gtest",
         "@com_google_googletest//:gtest_main",

test/bytecode_util_test.cc

@@ -59,7 +59,14 @@ TEST(TestBytecodeUtil, getFunctionNameIndex) {
   // OK.
   EXPECT_TRUE(BytecodeUtil::getFunctionNameIndex(source, actual));
   EXPECT_FALSE(actual.empty());
-  EXPECT_EQ(actual.find(0)->second, "proxy_abi_version_0_2_0");
+  bool abi_version_found = false;
+  for (auto it : actual) {
+    if (it.second == "proxy_abi_version_0_2_0") {
+      abi_version_found = true;
+      break;
+    }
+  }
+  EXPECT_TRUE(abi_version_found);
 
   // Fail due to the corrupted bytecode.
   // TODO(@mathetake): here we haven't covered all the parsing failure branches. Add more cases.