Skip to content

Commit e2733d2

Browse files
committed
Cleanup unused code - no longer reachable without pausing
1 parent 4649856 commit e2733d2

2 files changed

Lines changed: 11 additions & 72 deletions

File tree

‎src/node_http2.cc‎

Lines changed: 9 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -981,45 +981,23 @@ ssize_t Http2Session::OnMaxFrameSizePadding(size_t frameLen,
981981
// quite expensive. This is a potential performance optimization target later.
982982
void Http2Session::ConsumeHTTP2Data() {
983983
CHECK_NOT_NULL(stream_buf_.base);
984-
CHECK_LE(stream_buf_offset_, stream_buf_.len);
985-
size_t read_len = stream_buf_.len - stream_buf_offset_;
986984

987985
// multiple side effects.
988986
Debug(this, "receiving %d bytes [wants data? %d]",
989-
read_len,
987+
stream_buf_.len,
990988
nghttp2_session_want_read(session_.get()));
991-
set_receive_paused(false);
992989
custom_recv_error_code_ = nullptr;
993990
set_receiving();
994991
ssize_t ret =
995992
nghttp2_session_mem_recv(session_.get(),
996-
reinterpret_cast<uint8_t*>(stream_buf_.base) +
997-
stream_buf_offset_,
998-
read_len);
993+
reinterpret_cast<uint8_t*>(stream_buf_.base),
994+
stream_buf_.len);
999995
set_receiving(false);
1000996
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
1001997
CHECK_IMPLIES(custom_recv_error_code_ != nullptr, ret < 0);
1002998

1003-
if (is_receive_paused()) {
1004-
CHECK(is_reading_stopped());
1005-
1006-
CHECK_GT(ret, 0);
1007-
CHECK_LE(static_cast<size_t>(ret), read_len);
1008-
1009-
// Mark the remainder of the data as available for later consumption.
1010-
// Even if all bytes were received, a paused stream may delay the
1011-
// nghttp2_on_frame_recv_callback which may have an END_STREAM flag.
1012-
stream_buf_offset_ += ret;
1013-
// Still complete a Close() deferred during mem_recv; do not fall through
1014-
// to SendPendingData() here (paused receives historically skip that flush
1015-
// because a write may already be in progress).
1016-
MaybeFinishPendingClose();
1017-
goto done;
1018-
}
1019-
1020999
// We are done processing the current input chunk.
10211000
DecrementCurrentSessionMemory(stream_buf_.len);
1022-
stream_buf_offset_ = 0;
10231001
stream_buf_ab_.Reset();
10241002
stream_buf_allocation_.reset();
10251003
stream_buf_ = uv_buf_init(nullptr, 0);
@@ -1028,14 +1006,6 @@ void Http2Session::ConsumeHTTP2Data() {
10281006
// not written after pending RST_STREAM frames.
10291007
MaybeFinishPendingClose();
10301008

1031-
done:
1032-
// Finish a Close() deferred above before flushing, so GOAWAY is not written
1033-
// after pending RST_STREAM frames.
1034-
if (is_close_pending() && !is_destroyed()) {
1035-
set_close_pending(false);
1036-
FinishClose(pending_close_code_, pending_close_socket_closed_);
1037-
}
1038-
10391009
// Send any data that was queued up while processing the received data.
10401010
if (ret >= 0 && !is_destroyed()) {
10411011
SendPendingData();
@@ -1567,7 +1537,6 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
15671537
size_t offset = buf.base - session->stream_buf_.base;
15681538

15691539
// Verify that the data offset is inside the current read buffer.
1570-
CHECK_GE(offset, session->stream_buf_offset_);
15711540
CHECK_LE(offset, session->stream_buf_.len);
15721541
CHECK_LE(offset + buf.len, session->stream_buf_.len);
15731542

@@ -1882,11 +1851,6 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
18821851
return;
18831852
}
18841853

1885-
// If there is more incoming data queued up, consume it.
1886-
if (stream_buf_offset_ > 0) {
1887-
ConsumeHTTP2Data();
1888-
}
1889-
18901854
if (!is_write_scheduled() && !is_destroyed()) {
18911855
// Schedule a new write if nghttp2 wants to send data.
18921856
MaybeScheduleWrite();
@@ -2198,7 +2162,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
21982162
Context::Scope context_scope(env()->context());
21992163
Http2Scope h2scope(this);
22002164
CHECK_NOT_NULL(stream_);
2201-
Debug(this, "receiving %d bytes, offset %d", nread, stream_buf_offset_);
2165+
Debug(this, "receiving %d bytes", nread);
22022166
std::unique_ptr<BackingStore> bs = env()->release_managed_buffer(buf_);
22032167

22042168
// Only pass data on if nread > 0
@@ -2213,40 +2177,18 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
22132177

22142178
statistics_.data_received += nread;
22152179

2216-
if (stream_buf_offset_ == 0 && static_cast<size_t>(nread) != bs->ByteLength())
2217-
[[likely]] {
2180+
// ConsumeHTTP2Data() always consumes the whole chunk, so there is never a
2181+
// partially processed buffer left over from a previous read.
2182+
DCHECK_NULL(stream_buf_.base);
2183+
2184+
if (static_cast<size_t>(nread) != bs->ByteLength()) [[likely]] {
22182185
// Shrink to the actual amount of used data.
22192186
std::unique_ptr<BackingStore> old_bs = std::move(bs);
22202187
bs = ArrayBuffer::NewBackingStore(
22212188
env()->isolate(),
22222189
nread,
22232190
BackingStoreInitializationMode::kUninitialized);
22242191
memcpy(bs->Data(), old_bs->Data(), nread);
2225-
} else {
2226-
// This is a very unlikely case, and should only happen if the ReadStart()
2227-
// call in OnStreamAfterWrite() immediately provides data. If that does
2228-
// happen, we concatenate the data we received with the already-stored
2229-
// pending input data, slicing off the already processed part.
2230-
size_t pending_len = stream_buf_.len - stream_buf_offset_;
2231-
std::unique_ptr<BackingStore> new_bs = ArrayBuffer::NewBackingStore(
2232-
env()->isolate(),
2233-
pending_len + nread,
2234-
BackingStoreInitializationMode::kUninitialized);
2235-
memcpy(static_cast<char*>(new_bs->Data()),
2236-
stream_buf_.base + stream_buf_offset_,
2237-
pending_len);
2238-
memcpy(static_cast<char*>(new_bs->Data()) + pending_len,
2239-
bs->Data(),
2240-
nread);
2241-
2242-
bs = std::move(new_bs);
2243-
nread = bs->ByteLength();
2244-
stream_buf_offset_ = 0;
2245-
stream_buf_ab_.Reset();
2246-
2247-
// We have now fully processed the stream_buf_ input chunk (by moving the
2248-
// remaining part into buf, which will be accounted for below).
2249-
DecrementCurrentSessionMemory(stream_buf_.len);
22502192
}
22512193

22522194
IncrementCurrentSessionMemory(nread);

‎src/node_http2.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,8 @@ constexpr int kSessionStateClosing = 0x8;
8585
constexpr int kSessionStateSending = 0x10;
8686
constexpr int kSessionStateWriteInProgress = 0x20;
8787
constexpr int kSessionStateReadingStopped = 0x40;
88-
constexpr int kSessionStateReceivePaused = 0x80;
89-
constexpr int kSessionStateReceiving = 0x100;
90-
constexpr int kSessionStateClosePending = 0x200;
88+
constexpr int kSessionStateReceiving = 0x80;
89+
constexpr int kSessionStateClosePending = 0x100;
9190

9291
// The Padding Strategy determines the method by which extra padding is
9392
// selected for HEADERS and DATA frames. These are configurable via the
@@ -698,7 +697,6 @@ class Http2Session : public AsyncWrap,
698697
IS_FLAG(sending, kSessionStateSending)
699698
IS_FLAG(write_in_progress, kSessionStateWriteInProgress)
700699
IS_FLAG(reading_stopped, kSessionStateReadingStopped)
701-
IS_FLAG(receive_paused, kSessionStateReceivePaused)
702700
IS_FLAG(receiving, kSessionStateReceiving)
703701
IS_FLAG(close_pending, kSessionStateClosePending)
704702

@@ -979,7 +977,6 @@ class Http2Session : public AsyncWrap,
979977
// will be set. stream_buf_ab_ is lazily created from stream_buf_allocation_.
980978
v8::Global<v8::ArrayBuffer> stream_buf_ab_;
981979
std::unique_ptr<v8::BackingStore> stream_buf_allocation_;
982-
size_t stream_buf_offset_ = 0;
983980
// Custom error code for errors that originated inside one of the callbacks
984981
// called by nghttp2_session_mem_recv.
985982
const char* custom_recv_error_code_ = nullptr;

0 commit comments

Comments
 (0)