diff --git a/src/core/ext/filters/client_channel/subchannel_index.cc b/src/core/ext/filters/client_channel/subchannel_index.cc index f2b6c24e8ee..1c23a6c4be4 100644 --- a/src/core/ext/filters/client_channel/subchannel_index.cc +++ b/src/core/ext/filters/client_channel/subchannel_index.cc @@ -42,7 +42,7 @@ struct grpc_subchannel_key { grpc_subchannel_args args; }; -static gpr_atm g_force_creation = false; +static bool g_force_creation = false; static grpc_subchannel_key* create_key( const grpc_subchannel_args* args, @@ -74,7 +74,7 @@ static grpc_subchannel_key* subchannel_key_copy(grpc_subchannel_key* k) { int grpc_subchannel_key_compare(const grpc_subchannel_key* a, const grpc_subchannel_key* b) { // To pretend the keys are different, return a non-zero value. - if (GPR_UNLIKELY(gpr_atm_no_barrier_load(&g_force_creation))) return 1; + if (GPR_UNLIKELY(g_force_creation)) return 1; int c = GPR_ICMP(a->args.filter_count, b->args.filter_count); if (c != 0) return c; if (a->args.filter_count > 0) { @@ -251,5 +251,5 @@ void grpc_subchannel_index_unregister(grpc_subchannel_key* key, } void grpc_subchannel_index_test_only_set_force_creation(bool force_creation) { - gpr_atm_no_barrier_store(&g_force_creation, force_creation); + g_force_creation = force_creation; } diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index e5d6132012a..a9d68ab0582 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -119,6 +119,7 @@ class ClientLbEnd2endTest : public ::testing::Test { } void SetUp() override { + grpc_init(); response_generator_ = grpc_core::MakeRefCounted(); } @@ -127,6 +128,7 @@ class ClientLbEnd2endTest : public ::testing::Test { for (size_t i = 0; i < servers_.size(); ++i) { servers_[i]->Shutdown(); } + grpc_shutdown(); } void CreateServers(size_t num_servers, @@ -560,7 +562,23 @@ TEST_F(ClientLbEnd2endTest, PickFirstUpdateSuperset) { EXPECT_EQ("pick_first", channel->GetLoadBalancingPolicyName()); } -TEST_F(ClientLbEnd2endTest, PickFirstManyUpdates) { +class ClientLbEnd2endWithParamTest + : public ClientLbEnd2endTest, + public ::testing::WithParamInterface { + protected: + void SetUp() override { + grpc_subchannel_index_test_only_set_force_creation(GetParam()); + ClientLbEnd2endTest::SetUp(); + } + + void TearDown() override { + ClientLbEnd2endTest::TearDown(); + grpc_subchannel_index_test_only_set_force_creation(false); + } +}; + +TEST_P(ClientLbEnd2endWithParamTest, PickFirstManyUpdates) { + gpr_log(GPR_INFO, "subchannel force creation: %d", GetParam()); // Start servers and send one RPC per server. const int kNumServers = 3; StartServers(kNumServers); @@ -570,20 +588,21 @@ TEST_F(ClientLbEnd2endTest, PickFirstManyUpdates) { for (size_t i = 0; i < servers_.size(); ++i) { ports.emplace_back(servers_[i]->port_); } - for (const bool force_creation : {true, false}) { - grpc_subchannel_index_test_only_set_force_creation(force_creation); - gpr_log(GPR_INFO, "Force subchannel creation: %d", force_creation); - for (size_t i = 0; i < 1000; ++i) { - std::shuffle(ports.begin(), ports.end(), - std::mt19937(std::random_device()())); - SetNextResolution(ports); - if (i % 10 == 0) CheckRpcSendOk(stub, DEBUG_LOCATION); - } + for (size_t i = 0; i < 1000; ++i) { + std::shuffle(ports.begin(), ports.end(), + std::mt19937(std::random_device()())); + SetNextResolution(ports); + // We should re-enter core at the end of the loop to give the resolution + // setting closure a chance to run. + if ((i + 1) % 10 == 0) CheckRpcSendOk(stub, DEBUG_LOCATION); } // Check LB policy name for the channel. EXPECT_EQ("pick_first", channel->GetLoadBalancingPolicyName()); } +INSTANTIATE_TEST_CASE_P(SubchannelForceCreation, ClientLbEnd2endWithParamTest, + ::testing::Bool()); + TEST_F(ClientLbEnd2endTest, PickFirstReresolutionNoSelected) { // Prepare the ports for up servers and down servers. const int kNumServers = 3; @@ -984,8 +1003,6 @@ TEST_F(ClientLbEnd2endTest, RoundRobinSingleReconnect) { int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); grpc_test_init(argc, argv); - grpc_init(); const auto result = RUN_ALL_TESTS(); - grpc_shutdown(); return result; }