diff --git a/source/common/http/BUILD b/source/common/http/BUILD index e13f8a1fd16b8..165b250813acd 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -35,6 +35,15 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "async_client_utility_lib", + srcs = ["async_client_utility.cc"], + hdrs = ["async_client_utility.h"], + deps = [ + "//include/envoy/http:async_client_interface", + ], +) + envoy_cc_library( name = "codec_client_lib", srcs = ["codec_client.cc"], diff --git a/source/common/http/async_client_utility.cc b/source/common/http/async_client_utility.cc new file mode 100644 index 0000000000000..17124f06fb348 --- /dev/null +++ b/source/common/http/async_client_utility.cc @@ -0,0 +1,30 @@ +#include "common/http/async_client_utility.h" + +namespace Envoy { +namespace Http { + +AsyncClientRequestTracker::~AsyncClientRequestTracker() { + for (auto* active_request : active_requests_) { + active_request->cancel(); + } +} + +void AsyncClientRequestTracker::add(AsyncClient::Request& request) { + ASSERT(active_requests_.find(&request) == active_requests_.end(), "request is already tracked."); + active_requests_.insert(&request); +} + +void AsyncClientRequestTracker::remove(const AsyncClient::Request& request) { + // Notice that use of "const_cast" here is motivated by keeping API convenient for client code. + // In the context where remove() will be typically called, request.cancel() is no longer + // desirable and therefore get prevented by means of "const" modifier. + auto it = active_requests_.find(const_cast(&request)); + // Support a use case where request callbacks might get called prior to a request handle + // is returned from AsyncClient::send(). + if (it != active_requests_.end()) { + active_requests_.erase(it); + } +} + +} // namespace Http +} // namespace Envoy diff --git a/source/common/http/async_client_utility.h b/source/common/http/async_client_utility.h new file mode 100644 index 0000000000000..41a98873904c1 --- /dev/null +++ b/source/common/http/async_client_utility.h @@ -0,0 +1,43 @@ +#pragma once + +#include "envoy/http/async_client.h" + +namespace Envoy { +namespace Http { + +/** + * Keeps track of active async HTTP requests to be able to cancel them on destruction. + */ +class AsyncClientRequestTracker { +public: + /** + * Cancels all known active async HTTP requests. + */ + ~AsyncClientRequestTracker(); + /** + * Includes a given async HTTP request into a set of known active requests. + * @param request request handle + */ + void add(AsyncClient::Request& request); + /** + * Excludes a given async HTTP request from a set of known active requests. + * + * NOTE: Asymmetry between signatures of add() and remove() is caused by the difference + * between contexts in which these methods will be used. + * add() will be called right after AsyncClient::send() when request.cancel() is + * perfectly valid and desirable. + * However, remove() will be called in the context of + * AsyncClient::Callbacks::[onSuccess | onFailure] where request.cancel() is no longer + * expected and therefore get prevented by means of "const" modifier. + * + * @param request request handle + */ + void remove(const AsyncClient::Request& request); + +private: + // Track active async HTTP requests to be able to cancel them on destruction. + absl::flat_hash_set active_requests_; +}; + +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 593d22653e689..92360ccd4e3ee 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -36,6 +36,15 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "async_client_utility_test", + srcs = ["async_client_utility_test.cc"], + deps = [ + "//source/common/http:async_client_utility_lib", + "//test/mocks/http:http_mocks", + ], +) + envoy_cc_test( name = "codec_client_test", srcs = ["codec_client_test.cc"], diff --git a/test/common/http/async_client_utility_test.cc b/test/common/http/async_client_utility_test.cc new file mode 100644 index 0000000000000..afcfc0ef0d0e2 --- /dev/null +++ b/test/common/http/async_client_utility_test.cc @@ -0,0 +1,54 @@ +#include "common/http/async_client_utility.h" + +#include "test/mocks/http/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::NiceMock; +using testing::StrictMock; + +namespace Envoy { +namespace Http { +namespace { + +class AsyncClientRequestTrackerTest : public testing::Test { +public: + std::unique_ptr active_requests_{ + std::make_unique()}; + + NiceMock async_client_; + StrictMock request1_{&async_client_}; + StrictMock request2_{&async_client_}; + StrictMock request3_{&async_client_}; +}; + +TEST_F(AsyncClientRequestTrackerTest, ShouldSupportRemoveWithoutAdd) { + // Should not fail. + active_requests_->remove(request1_); +} + +TEST_F(AsyncClientRequestTrackerTest, OnDestructDoNothingIfThereAreNoActiveRequests) { + // Trigger destruction. + active_requests_.reset(); +} + +TEST_F(AsyncClientRequestTrackerTest, OnDestructCancelActiveRequests) { + // Include active requests. + active_requests_->add(request1_); + active_requests_->add(request2_); + active_requests_->add(request3_); + // Exclude active requests. + active_requests_->remove(request2_); + + // Must cancel active requests on destruction. + EXPECT_CALL(request1_, cancel()); + EXPECT_CALL(request3_, cancel()); + + // Trigger destruction. + active_requests_.reset(); +} + +} // namespace +} // namespace Http +} // namespace Envoy