Skip to content

Commit e744e5b

Browse files
backport to v1.13: test flake: fix flake in ApiListener integration test (envoyproxy#9808) (envoyproxy#13316)
Description: previously the test was not waiting for the expectation on the server's thread to complete. Therefore, there was a use after free race condition with the ApiListener's TlsCachingDateProvider. This PR makes it so that the test waits for the expectation to be fulfilled and thus prevents the race. Risk Level: low Testing: ran integration test a few thousand times locally on a linux machine. Signed-off-by: Jose Nino <[email protected]> Signed-off-by: Antonio Vicente <[email protected]>
1 parent 4e32599 commit e744e5b

File tree

3 files changed

+14
-7
lines changed

3 files changed

+14
-7
lines changed

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.13.5
1+
1.13.6

docs/root/intro/version_history.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
Version history
22
---------------
33

4+
1.13.6 (September 29, 2020)
5+
===========================
6+
Changes
7+
-------
8+
* test: fix flaky test.
9+
410
1.13.5 (September 29, 2020)
511
===========================
612
Changes

test/integration/api_listener_integration_test.cc

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "test/test_common/environment.h"
55
#include "test/test_common/utility.h"
66

7+
#include "absl/synchronization/notification.h"
78
#include "gmock/gmock.h"
89
#include "gtest/gtest.h"
910

@@ -76,13 +77,15 @@ name: api_listener
7677
NiceMock<Http::MockStreamEncoder> stream_encoder_;
7778
};
7879

80+
ACTION_P(Notify, notification) { notification->Notify(); }
81+
7982
INSTANTIATE_TEST_SUITE_P(IpVersions, ApiListenerIntegrationTest,
8083
testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
8184
TestUtility::ipTestParamsToString);
8285

8386
TEST_P(ApiListenerIntegrationTest, Basic) {
84-
ConditionalInitializer test_ran;
85-
test_server_->server().dispatcher().post([this, &test_ran]() -> void {
87+
absl::Notification done;
88+
test_server_->server().dispatcher().post([this, &done]() -> void {
8689
ASSERT_TRUE(test_server_->server().listenerManager().apiListener().has_value());
8790
ASSERT_EQ("api_listener", test_server_->server().listenerManager().apiListener()->get().name());
8891
ASSERT_TRUE(test_server_->server().listenerManager().apiListener()->get().http().has_value());
@@ -97,17 +100,15 @@ TEST_P(ApiListenerIntegrationTest, Basic) {
97100
Http::TestHeaderMapImpl expected_response_headers{{":status", "200"}};
98101
EXPECT_CALL(stream_encoder_, encodeHeaders(_, false));
99102
EXPECT_CALL(stream_encoder_, encodeData(_, false));
100-
EXPECT_CALL(stream_encoder_, encodeData(BufferStringEqual(""), true));
103+
EXPECT_CALL(stream_encoder_, encodeData(BufferStringEqual(""), true)).WillOnce(Notify(&done));
101104

102105
// Send a headers-only request
103106
stream_decoder.decodeHeaders(
104107
Http::HeaderMapPtr(new Http::TestHeaderMapImpl{
105108
{":method", "GET"}, {":path", "/api"}, {":scheme", "http"}, {":authority", "host"}}),
106109
true);
107-
108-
test_ran.setReady();
109110
});
110-
test_ran.waitReady();
111+
ASSERT_TRUE(done.WaitForNotificationWithTimeout(absl::Seconds(1)));
111112
}
112113

113114
} // namespace

0 commit comments

Comments
 (0)