Adding test to make sure that http2 transport gets cleaned up (#25714)

* Add test to make sure that transports get destroyed

* Reviewer comments
reviewable/pr25108/r13^2
Yash Tibrewal 4 years ago committed by GitHub
parent 949c37c884
commit e9de13e6ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 26
      src/core/ext/transport/chttp2/transport/chttp2_transport.cc
  2. 11
      src/core/ext/transport/chttp2/transport/chttp2_transport.h
  3. 111
      test/core/transport/chttp2/too_many_pings_test.cc

@ -165,6 +165,26 @@ static void reset_byte_stream(void* arg, grpc_error* error);
// GRPC_EXPERIMENTAL_DISABLE_FLOW_CONTROL
bool g_flow_control_enabled = true;
namespace grpc_core {
namespace {
TestOnlyGlobalHttp2TransportInitCallback test_only_init_callback = nullptr;
TestOnlyGlobalHttp2TransportDestructCallback test_only_destruct_callback =
nullptr;
} // namespace
void TestOnlySetGlobalHttp2TransportInitCallback(
TestOnlyGlobalHttp2TransportInitCallback callback) {
test_only_init_callback = callback;
}
void TestOnlySetGlobalHttp2TransportDestructCallback(
TestOnlyGlobalHttp2TransportDestructCallback callback) {
test_only_destruct_callback = callback;
}
} // namespace grpc_core
//
// CONSTRUCTION/DESTRUCTION/REFCOUNTING
//
@ -221,6 +241,9 @@ grpc_chttp2_transport::~grpc_chttp2_transport() {
GRPC_ERROR_UNREF(closed_with_error);
gpr_free(ping_acks);
if (grpc_core::test_only_destruct_callback != nullptr) {
grpc_core::test_only_destruct_callback();
}
}
static const grpc_transport_vtable* get_vtable(void);
@ -506,6 +529,9 @@ grpc_chttp2_transport::grpc_chttp2_transport(
grpc_chttp2_initiate_write(this, GRPC_CHTTP2_INITIATE_WRITE_INITIAL_WRITE);
post_benign_reclaimer(this);
if (grpc_core::test_only_init_callback != nullptr) {
grpc_core::test_only_init_callback();
}
}
static void destroy_transport_locked(void* tp, grpc_error* /*error*/) {

@ -49,4 +49,15 @@ void grpc_chttp2_transport_start_reading(
grpc_transport* transport, grpc_slice_buffer* read_buffer,
grpc_closure* notify_on_receive_settings);
namespace grpc_core {
typedef void (*TestOnlyGlobalHttp2TransportInitCallback)();
typedef void (*TestOnlyGlobalHttp2TransportDestructCallback)();
void TestOnlySetGlobalHttp2TransportInitCallback(
TestOnlyGlobalHttp2TransportInitCallback callback);
void TestOnlySetGlobalHttp2TransportDestructCallback(
TestOnlyGlobalHttp2TransportDestructCallback callback);
} // namespace grpc_core
#endif /* GRPC_CORE_EXT_TRANSPORT_CHTTP2_TRANSPORT_CHTTP2_TRANSPORT_H */

@ -40,6 +40,7 @@
#include <grpcpp/server_builder.h>
#include "src/core/ext/filters/client_channel/resolver/fake/fake_resolver.h"
#include "src/core/ext/transport/chttp2/transport/chttp2_transport.h"
#include "src/core/lib/gpr/useful.h"
#include "src/core/lib/gprpp/host_port.h"
#include "src/core/lib/gprpp/thd.h"
@ -59,6 +60,48 @@
namespace {
class TransportCounter {
public:
static void CounterInitCallback() {
absl::MutexLock lock(&mu());
++count_;
}
static void CounterDestructCallback() {
absl::MutexLock lock(&mu());
if (--count_ == 0) {
cv().SignalAll();
}
}
static void WaitForTransportsToBeDestroyed() {
absl::MutexLock lock(&mu());
while (count_ != 0) {
ASSERT_FALSE(cv().WaitWithTimeout(&mu(), absl::Seconds(10)));
}
}
static int count() {
absl::MutexLock lock(&mu());
return count_;
}
static absl::Mutex& mu() {
static absl::Mutex* mu = new absl::Mutex();
return *mu;
}
static absl::CondVar& cv() {
static absl::CondVar* cv = new absl::CondVar();
return *cv;
}
private:
static int count_;
};
int TransportCounter::count_ = 0;
void* tag(intptr_t t) { return reinterpret_cast<void*>(t); }
// Perform a simple RPC where the server cancels the request with
@ -681,8 +724,8 @@ TEST(TooManyPings, BdpPingNotSentWithoutReceiveSideActivity) {
grpc_channel* channel = grpc_insecure_channel_create(
server_address.c_str(), &client_channel_args, nullptr);
VerifyChannelReady(channel, cq);
EXPECT_EQ(TransportCounter::count(), 2 /* one each for server and client */);
cq_verifier* cqv = cq_verifier_create(cq);
cq_verify_empty_timeout(cqv, 1);
// Channel should be able to send two pings without disconnect if there was no
// BDP sent.
grpc_channel_ping(channel, cq, tag(1), nullptr);
@ -707,9 +750,67 @@ TEST(TooManyPings, BdpPingNotSentWithoutReceiveSideActivity) {
grpc_channel_ping(channel, cq, tag(4), nullptr);
CQ_EXPECT_COMPLETION(cqv, tag(4), 1);
cq_verify(cqv, 5);
// Give some time for the server to disconnect if it hasn't already.
cq_verify_empty_timeout(cqv, 3);
// Make sure that the transports have been destroyed
VerifyChannelDisconnected(channel, cq);
TransportCounter::WaitForTransportsToBeDestroyed();
cq_verifier_destroy(cqv);
// shutdown and destroy the client and server
ServerShutdownAndDestroy(server, cq);
grpc_channel_destroy(channel);
grpc_completion_queue_shutdown(cq);
while (grpc_completion_queue_next(cq, gpr_inf_future(GPR_CLOCK_REALTIME),
nullptr)
.type != GRPC_QUEUE_SHUTDOWN) {
}
grpc_completion_queue_destroy(cq);
}
TEST(TooManyPings, TransportsGetCleanedUpOnDisconnect) {
grpc_completion_queue* cq = grpc_completion_queue_create_for_next(nullptr);
// create the client and server
std::string server_address =
grpc_core::JoinHostPort("localhost", grpc_pick_unused_port_or_die());
grpc_arg server_args[] = {
grpc_channel_arg_integer_create(
const_cast<char*>(
GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS),
60 * 1000),
grpc_channel_arg_integer_create(
const_cast<char*>(GRPC_ARG_HTTP2_MAX_PING_STRIKES), 1)};
grpc_channel_args server_channel_args = {GPR_ARRAY_SIZE(server_args),
server_args};
grpc_server* server = grpc_server_create(&server_channel_args, nullptr);
grpc_server_register_completion_queue(server, cq, nullptr);
GPR_ASSERT(
grpc_server_add_insecure_http2_port(server, server_address.c_str()));
grpc_server_start(server);
grpc_arg client_args[] = {
grpc_channel_arg_integer_create(
const_cast<char*>(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA), 0),
grpc_channel_arg_integer_create(
const_cast<char*>(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS), 1)};
grpc_channel_args client_channel_args = {GPR_ARRAY_SIZE(client_args),
client_args};
grpc_channel* channel = grpc_insecure_channel_create(
server_address.c_str(), &client_channel_args, nullptr);
VerifyChannelReady(channel, cq);
EXPECT_EQ(TransportCounter::count(), 2 /* one each for server and client */);
cq_verifier* cqv = cq_verifier_create(cq);
// First ping
grpc_channel_ping(channel, cq, tag(1), nullptr);
CQ_EXPECT_COMPLETION(cqv, tag(1), 1);
cq_verify(cqv, 5);
// Second ping
grpc_channel_ping(channel, cq, tag(2), nullptr);
CQ_EXPECT_COMPLETION(cqv, tag(2), 1);
cq_verify(cqv, 5);
// Third ping caused disconnect
grpc_channel_ping(channel, cq, tag(2), nullptr);
CQ_EXPECT_COMPLETION(cqv, tag(2), 1);
cq_verify(cqv, 5);
// Make sure that the transports have been destroyed
VerifyChannelDisconnected(channel, cq);
TransportCounter::WaitForTransportsToBeDestroyed();
cq_verifier_destroy(cqv);
// shutdown and destroy the client and server
ServerShutdownAndDestroy(server, cq);
@ -727,6 +828,10 @@ TEST(TooManyPings, BdpPingNotSentWithoutReceiveSideActivity) {
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
grpc::testing::TestEnvironment env(argc, argv);
grpc_core::TestOnlySetGlobalHttp2TransportInitCallback(
TransportCounter::CounterInitCallback);
grpc_core::TestOnlySetGlobalHttp2TransportDestructCallback(
TransportCounter::CounterDestructCallback);
grpc_init();
auto result = RUN_ALL_TESTS();
grpc_shutdown();

Loading…
Cancel
Save