Merge pull request envoyproxy#478 from jplevyak/http-reentrant

Prevent reentrant HttpCall callbacks.
lizan · Apr 14, 2020 · 6f9ca9d · 6f9ca9d
2 parents 686a2e1 + cc7d1d8
commit 6f9ca9d
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc
@@ -1589,7 +1589,15 @@ void Context::setEncoderFilterCallbacks(Envoy::Http::StreamEncoderFilterCallback
   encoder_callbacks_ = &callbacks;
 }
 
-void Context::onHttpCallSuccess(uint32_t token, Envoy::Http::ResponseMessagePtr& response) {
+void Context::onHttpCallSuccess(uint32_t token, Envoy::Http::ResponseMessagePtr&& response) {
+  // TODO: convert this into a function in proxy-wasm-cpp-host and use here.
+  if (proxy_wasm::current_context_ != nullptr) {
+    // We are in a reentrant call, so defer.
+    wasm()->addAfterVmCallAction([this, token, response = response.release()] {
+      onHttpCallSuccess(token, std::unique_ptr<Envoy::Http::ResponseMessage>(response));
+    });
+    return;
+  }
   http_call_response_ = &response;
   uint32_t body_size = response->body() ? response->body()->length() : 0;
   onHttpCallResponse(token, response->headers().size(), body_size,
@@ -1599,6 +1607,11 @@ void Context::onHttpCallSuccess(uint32_t token, Envoy::Http::ResponseMessagePtr&
 }
 
 void Context::onHttpCallFailure(uint32_t token, Http::AsyncClient::FailureReason reason) {
+  if (proxy_wasm::current_context_ != nullptr) {
+    // We are in a reentrant call, so defer.
+    wasm()->addAfterVmCallAction([this, token, reason] { onHttpCallFailure(token, reason); });
+    return;
+  }
   status_code_ = static_cast<uint32_t>(WasmResult::BrokenConnection);
   // This is the only value currently.
   ASSERT(reason == Http::AsyncClient::FailureReason::Reset);
@@ -1609,6 +1622,7 @@ void Context::onHttpCallFailure(uint32_t token, Http::AsyncClient::FailureReason
 }
 
 void Context::onGrpcReceiveWrapper(uint32_t token, ::Envoy::Buffer::InstancePtr response) {
+  ASSERT(proxy_wasm::current_context_ == nullptr); // Non-reentrant.
   if (wasm()->on_grpc_receive_) {
     grpc_receive_buffer_ = std::move(response);
     uint32_t response_size = grpc_receive_buffer_->length();
@@ -1622,6 +1636,13 @@ void Context::onGrpcReceiveWrapper(uint32_t token, ::Envoy::Buffer::InstancePtr
 
 void Context::onGrpcCloseWrapper(uint32_t token, const Grpc::Status::GrpcStatus& status,
                                  const absl::string_view message) {
+  if (proxy_wasm::current_context_ != nullptr) {
+    // We are in a reentrant call, so defer.
+    wasm()->addAfterVmCallAction([this, token, status, message = std::string(message)] {
+      onGrpcCloseWrapper(token, status, message);
+    });
+    return;
+  }
   if (wasm()->on_grpc_close_) {
     status_code_ = static_cast<uint32_t>(status);
     status_message_ = message;

diff --git a/source/extensions/common/wasm/context.h b/source/extensions/common/wasm/context.h
@@ -300,7 +300,7 @@ class Context : public proxy_wasm::ContextBase,
   struct AsyncClientHandler : public Http::AsyncClient::Callbacks {
     // Http::AsyncClient::Callbacks
     void onSuccess(Envoy::Http::ResponseMessagePtr&& response) override {
-      context_->onHttpCallSuccess(token_, response);
+      context_->onHttpCallSuccess(token_, std::move(response));
     }
     void onFailure(Http::AsyncClient::FailureReason reason) override {
       context_->onHttpCallFailure(token_, reason);
@@ -353,7 +353,7 @@ class Context : public proxy_wasm::ContextBase,
     Grpc::RawAsyncStream* stream_;
   };
 
-  void onHttpCallSuccess(uint32_t token, Envoy::Http::ResponseMessagePtr& response);
+  void onHttpCallSuccess(uint32_t token, Envoy::Http::ResponseMessagePtr&& response);
   void onHttpCallFailure(uint32_t token, Http::AsyncClient::FailureReason reason);
 
   void onGrpcCreateInitialMetadata(uint32_t token, Http::RequestHeaderMap& metadata);