Cleanup unused code - no longer reachable without pausing

Author: pimterry

src/node_http2.cc

@@ -981,45 +981,24 @@ ssize_t Http2Session::OnMaxFrameSizePadding(size_t frameLen,
 // quite expensive. This is a potential performance optimization target later.
 void Http2Session::ConsumeHTTP2Data() {
   CHECK_NOT_NULL(stream_buf_.base);
-  CHECK_LE(stream_buf_offset_, stream_buf_.len);
-  size_t read_len = stream_buf_.len - stream_buf_offset_;
 
   // multiple side effects.
-  Debug(this, "receiving %d bytes [wants data? %d]",
-        read_len,
+  Debug(this,
+        "receiving %d bytes [wants data? %d]",
+        stream_buf_.len,
         nghttp2_session_want_read(session_.get()));
-  set_receive_paused(false);
   custom_recv_error_code_ = nullptr;
   set_receiving();
   ssize_t ret =
-    nghttp2_session_mem_recv(session_.get(),
-                             reinterpret_cast<uint8_t*>(stream_buf_.base) +
-                                 stream_buf_offset_,
-                             read_len);
+      nghttp2_session_mem_recv(session_.get(),
+                               reinterpret_cast<uint8_t*>(stream_buf_.base),
+                               stream_buf_.len);
   set_receiving(false);
   CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
   CHECK_IMPLIES(custom_recv_error_code_ != nullptr, ret < 0);
 
-  if (is_receive_paused()) {
-    CHECK(is_reading_stopped());
-
-    CHECK_GT(ret, 0);
-    CHECK_LE(static_cast<size_t>(ret), read_len);
-
-    // Mark the remainder of the data as available for later consumption.
-    // Even if all bytes were received, a paused stream may delay the
-    // nghttp2_on_frame_recv_callback which may have an END_STREAM flag.
-    stream_buf_offset_ += ret;
-    // Still complete a Close() deferred during mem_recv; do not fall through
-    // to SendPendingData() here (paused receives historically skip that flush
-    // because a write may already be in progress).
-    MaybeFinishPendingClose();
-    goto done;
-  }
-
   // We are done processing the current input chunk.
   DecrementCurrentSessionMemory(stream_buf_.len);
-  stream_buf_offset_ = 0;
   stream_buf_ab_.Reset();
   stream_buf_allocation_.reset();
   stream_buf_ = uv_buf_init(nullptr, 0);
@@ -1028,14 +1007,6 @@ void Http2Session::ConsumeHTTP2Data() {
   // not written after pending RST_STREAM frames.
   MaybeFinishPendingClose();
 
-done:
-  // Finish a Close() deferred above before flushing, so GOAWAY is not written
-  // after pending RST_STREAM frames.
-  if (is_close_pending() && !is_destroyed()) {
-    set_close_pending(false);
-    FinishClose(pending_close_code_, pending_close_socket_closed_);
-  }
-
   // Send any data that was queued up while processing the received data.
   if (ret >= 0 && !is_destroyed()) {
     SendPendingData();
@@ -1567,7 +1538,6 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
   size_t offset = buf.base - session->stream_buf_.base;
 
   // Verify that the data offset is inside the current read buffer.
-  CHECK_GE(offset, session->stream_buf_offset_);
   CHECK_LE(offset, session->stream_buf_.len);
   CHECK_LE(offset + buf.len, session->stream_buf_.len);
 
@@ -1882,11 +1852,6 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
     return;
   }
 
-  // If there is more incoming data queued up, consume it.
-  if (stream_buf_offset_ > 0) {
-    ConsumeHTTP2Data();
-  }
-
   if (!is_write_scheduled() && !is_destroyed()) {
     // Schedule a new write if nghttp2 wants to send data.
     MaybeScheduleWrite();
@@ -2198,7 +2163,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
   Context::Scope context_scope(env()->context());
   Http2Scope h2scope(this);
   CHECK_NOT_NULL(stream_);
-  Debug(this, "receiving %d bytes, offset %d", nread, stream_buf_offset_);
+  Debug(this, "receiving %d bytes", nread);
   std::unique_ptr<BackingStore> bs = env()->release_managed_buffer(buf_);
 
   // Only pass data on if nread > 0
@@ -2213,40 +2178,18 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
 
   statistics_.data_received += nread;
 
-  if (stream_buf_offset_ == 0 && static_cast<size_t>(nread) != bs->ByteLength())
-      [[likely]] {
+  // ConsumeHTTP2Data() always consumes the whole chunk, so there is never a
+  // partially processed buffer left over from a previous read.
+  DCHECK_NULL(stream_buf_.base);
+
+  if (static_cast<size_t>(nread) != bs->ByteLength()) [[likely]] {
     // Shrink to the actual amount of used data.
     std::unique_ptr<BackingStore> old_bs = std::move(bs);
     bs = ArrayBuffer::NewBackingStore(
         env()->isolate(),
         nread,
         BackingStoreInitializationMode::kUninitialized);
     memcpy(bs->Data(), old_bs->Data(), nread);
-  } else {
-    // This is a very unlikely case, and should only happen if the ReadStart()
-    // call in OnStreamAfterWrite() immediately provides data. If that does
-    // happen, we concatenate the data we received with the already-stored
-    // pending input data, slicing off the already processed part.
-    size_t pending_len = stream_buf_.len - stream_buf_offset_;
-    std::unique_ptr<BackingStore> new_bs = ArrayBuffer::NewBackingStore(
-        env()->isolate(),
-        pending_len + nread,
-        BackingStoreInitializationMode::kUninitialized);
-    memcpy(static_cast<char*>(new_bs->Data()),
-           stream_buf_.base + stream_buf_offset_,
-           pending_len);
-    memcpy(static_cast<char*>(new_bs->Data()) + pending_len,
-           bs->Data(),
-           nread);
-
-    bs = std::move(new_bs);
-    nread = bs->ByteLength();
-    stream_buf_offset_ = 0;
-    stream_buf_ab_.Reset();
-
-    // We have now fully processed the stream_buf_ input chunk (by moving the
-    // remaining part into buf, which will be accounted for below).
-    DecrementCurrentSessionMemory(stream_buf_.len);
   }
 
   IncrementCurrentSessionMemory(nread);

src/node_http2.h

@@ -85,9 +85,8 @@ constexpr int kSessionStateClosing = 0x8;
 constexpr int kSessionStateSending = 0x10;
 constexpr int kSessionStateWriteInProgress = 0x20;
 constexpr int kSessionStateReadingStopped = 0x40;
-constexpr int kSessionStateReceivePaused = 0x80;
-constexpr int kSessionStateReceiving = 0x100;
-constexpr int kSessionStateClosePending = 0x200;
+constexpr int kSessionStateReceiving = 0x80;
+constexpr int kSessionStateClosePending = 0x100;
 
 // The Padding Strategy determines the method by which extra padding is
 // selected for HEADERS and DATA frames. These are configurable via the
@@ -698,7 +697,6 @@ class Http2Session : public AsyncWrap,
   IS_FLAG(sending, kSessionStateSending)
   IS_FLAG(write_in_progress, kSessionStateWriteInProgress)
   IS_FLAG(reading_stopped, kSessionStateReadingStopped)
-  IS_FLAG(receive_paused, kSessionStateReceivePaused)
   IS_FLAG(receiving, kSessionStateReceiving)
   IS_FLAG(close_pending, kSessionStateClosePending)
 
@@ -979,7 +977,6 @@ class Http2Session : public AsyncWrap,
   // will be set. stream_buf_ab_ is lazily created from stream_buf_allocation_.
   v8::Global<v8::ArrayBuffer> stream_buf_ab_;
   std::unique_ptr<v8::BackingStore> stream_buf_allocation_;
-  size_t stream_buf_offset_ = 0;
   // Custom error code for errors that originated inside one of the callbacks
   // called by nghttp2_session_mem_recv.
   const char* custom_recv_error_code_ = nullptr;