Deletes a stream entry from CallbackVisitor's stream map in OnCloseStream().
This should fix the memory consumption bug reported in Envoy:
https://github.com/envoyproxy/envoy/issues/19761
PiperOrigin-RevId: 425673006
diff --git a/http2/adapter/callback_visitor.cc b/http2/adapter/callback_visitor.cc
index 1dd082e..7d59c6d 100644
--- a/http2/adapter/callback_visitor.cc
+++ b/http2/adapter/callback_visitor.cc
@@ -262,6 +262,7 @@
callbacks_->on_stream_close_callback(
nullptr, stream_id, static_cast<uint32_t>(error_code), user_data_);
}
+ stream_map_.erase(stream_id);
}
void CallbackVisitor::OnPriorityForStream(Http2StreamId /*stream_id*/,
diff --git a/http2/adapter/callback_visitor.h b/http2/adapter/callback_visitor.h
index fa0c770..1f7452b 100644
--- a/http2/adapter/callback_visitor.h
+++ b/http2/adapter/callback_visitor.h
@@ -69,6 +69,8 @@
bool OnMetadataEndForStream(Http2StreamId stream_id) override;
void OnErrorDebug(absl::string_view message) override;
+ size_t stream_map_size() const { return stream_map_.size(); }
+
private:
struct QUICHE_EXPORT_PRIVATE StreamInfo {
bool before_sent_headers = false;
@@ -82,9 +84,12 @@
void PopulateFrame(nghttp2_frame& frame, uint8_t frame_type,
Http2StreamId stream_id, size_t length, uint8_t flags,
uint32_t error_code, bool sent_headers);
+
// Creates the StreamInfoMap entry if it doesn't exist.
StreamInfoMap::iterator GetStreamInfo(Http2StreamId stream_id);
+ StreamInfoMap stream_map_;
+
Perspective perspective_;
nghttp2_session_callbacks_unique_ptr callbacks_;
void* user_data_;
@@ -92,8 +97,6 @@
nghttp2_frame current_frame_;
std::vector<nghttp2_settings_entry> settings_;
size_t remaining_data_ = 0;
-
- StreamInfoMap stream_map_;
};
} // namespace adapter
diff --git a/http2/adapter/callback_visitor_test.cc b/http2/adapter/callback_visitor_test.cc
index 9ed28f0..8f83da5 100644
--- a/http2/adapter/callback_visitor_test.cc
+++ b/http2/adapter/callback_visitor_test.cc
@@ -70,6 +70,8 @@
EXPECT_CALL(callbacks, OnFrameRecv(IsGoAway(5, NGHTTP2_ENHANCE_YOUR_CALM,
"calm down!!")));
visitor.OnGoAway(5, Http2ErrorCode::ENHANCE_YOUR_CALM, "calm down!!");
+
+ EXPECT_EQ(visitor.stream_map_size(), 0);
}
TEST(ClientCallbackVisitorUnitTest, StreamFrames) {
@@ -79,6 +81,8 @@
testing::InSequence seq;
+ EXPECT_EQ(visitor.stream_map_size(), 0);
+
// HEADERS on stream 1
EXPECT_CALL(callbacks, OnBeginFrame(HasFrameHeader(1, HEADERS, _)));
visitor.OnFrameHeader(1, 23, HEADERS, 4);
@@ -87,6 +91,8 @@
OnBeginHeaders(IsHeaders(1, _, NGHTTP2_HCAT_RESPONSE)));
visitor.OnBeginHeadersForStream(1);
+ EXPECT_EQ(visitor.stream_map_size(), 1);
+
EXPECT_CALL(callbacks, OnHeader(_, ":status", "200", _));
visitor.OnHeaderForStream(1, ":status", "200");
@@ -129,6 +135,9 @@
EXPECT_CALL(callbacks, OnBeginFrame(HasFrameHeader(3, RST_STREAM, 0)));
visitor.OnFrameHeader(3, 4, RST_STREAM, 0);
+ // No change in stream map size.
+ EXPECT_EQ(visitor.stream_map_size(), 1);
+
EXPECT_CALL(callbacks, OnFrameRecv(IsRstStream(3, NGHTTP2_INTERNAL_ERROR)));
visitor.OnRstStream(3, Http2ErrorCode::INTERNAL_ERROR);
@@ -147,6 +156,9 @@
EXPECT_CALL(callbacks, OnStreamClose(1, NGHTTP2_NO_ERROR));
visitor.OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR);
+ // Stream map is empty again.
+ EXPECT_EQ(visitor.stream_map_size(), 0);
+
EXPECT_CALL(callbacks, OnBeginFrame(HasFrameHeader(5, RST_STREAM, _)));
visitor.OnFrameHeader(5, 4, RST_STREAM, 0);
@@ -155,6 +167,8 @@
EXPECT_CALL(callbacks, OnStreamClose(5, NGHTTP2_REFUSED_STREAM));
visitor.OnCloseStream(5, Http2ErrorCode::REFUSED_STREAM);
+
+ EXPECT_EQ(visitor.stream_map_size(), 0);
}
TEST(ClientCallbackVisitorUnitTest, HeadersWithContinuation) {
@@ -228,6 +242,8 @@
EXPECT_CALL(callbacks, OnFrameRecv(IsPingAck(247)));
visitor.OnPing(247, true);
+
+ EXPECT_EQ(visitor.stream_map_size(), 0);
}
TEST(ServerCallbackVisitorUnitTest, StreamFrames) {
@@ -246,6 +262,8 @@
NGHTTP2_HCAT_REQUEST)));
visitor.OnBeginHeadersForStream(1);
+ EXPECT_EQ(visitor.stream_map_size(), 1);
+
EXPECT_CALL(callbacks, OnHeader(_, ":method", "POST", _));
visitor.OnHeaderForStream(1, ":method", "POST");
@@ -280,6 +298,8 @@
EXPECT_CALL(callbacks, OnStreamClose(1, NGHTTP2_NO_ERROR));
visitor.OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR);
+ EXPECT_EQ(visitor.stream_map_size(), 0);
+
// RST_STREAM on stream 3
EXPECT_CALL(callbacks, OnBeginFrame(HasFrameHeader(3, RST_STREAM, 0)));
visitor.OnFrameHeader(3, 4, RST_STREAM, 0);
@@ -289,6 +309,8 @@
EXPECT_CALL(callbacks, OnStreamClose(3, NGHTTP2_INTERNAL_ERROR));
visitor.OnCloseStream(3, Http2ErrorCode::INTERNAL_ERROR);
+
+ EXPECT_EQ(visitor.stream_map_size(), 0);
}
TEST(ServerCallbackVisitorUnitTest, DataWithPadding) {
