You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2017/12/13 00:12:57 UTC

[trafficserver] 01/02: Revert "Cancel closing if Http2SendDataFrameResult is NO_WINDOW"

This is an automated email from the ASF dual-hosted git repository.

masaori pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 9adddd6096cc213225e7ee20778645aa0b0d24ba
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Mon Nov 27 09:10:52 2017 +0900

    Revert "Cancel closing if Http2SendDataFrameResult is NO_WINDOW"
    
    This reverts commit 9a93d32863364e01267b5aabd322a9740c695441.
---
 proxy/http2/Http2ConnectionState.cc | 38 +++++++++++++++++++------------------
 proxy/http2/Http2ConnectionState.h  | 14 +++++++-------
 proxy/http2/Http2Stream.cc          | 22 ++++++++-------------
 3 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 6d76e2d..26fa97b 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1236,11 +1236,11 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
   ink_release_assert(stream != nullptr);
   Http2StreamDebug(ua_session, stream->get_id(), "top node, point=%d", node->point);
 
-  size_t len                      = 0;
-  Http2SendDataFrameResult result = send_a_data_frame(stream, len);
+  size_t len                       = 0;
+  Http2SendADataFrameResult result = send_a_data_frame(stream, len);
 
   switch (result) {
-  case Http2SendDataFrameResult::NO_ERROR: {
+  case HTTP2_SEND_A_DATA_FRAME_NO_ERROR: {
     // No response body to send
     if (len == 0 && !stream->is_body_done()) {
       dependency_tree->deactivate(node, len);
@@ -1249,7 +1249,7 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
     }
     break;
   }
-  case Http2SendDataFrameResult::DONE: {
+  case HTTP2_SEND_A_DATA_FRAME_DONE: {
     dependency_tree->deactivate(node, len);
     delete_stream(stream);
     break;
@@ -1264,7 +1264,7 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
   return;
 }
 
-Http2SendDataFrameResult
+Http2SendADataFrameResult
 Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_length)
 {
   const ssize_t window_size         = std::min(this->client_rwnd, stream->client_rwnd);
@@ -1286,7 +1286,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
   if (read_available_size > 0) {
     // We only need to check for window size when there is a payload
     if (window_size <= 0) {
-      return Http2SendDataFrameResult::NO_WINDOW;
+      return HTTP2_SEND_A_DATA_FRAME_NO_WINDOW;
     }
     // Copy into the payload buffer. Seems like we should be able to skip this copy step
     payload_length = current_reader->read(payload_buffer, write_available_size);
@@ -1298,7 +1298,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
   // If we return here, we never send the END_STREAM in the case of a early terminating OS.
   // OK if there is no body yet. Otherwise continue on to send a DATA frame and delete the stream
   if (!stream->is_body_done() && payload_length == 0) {
-    return Http2SendDataFrameResult::NO_PAYLOAD;
+    return HTTP2_SEND_A_DATA_FRAME_NO_PAYLOAD;
   }
 
   if (stream->is_body_done() && read_available_size <= write_available_size) {
@@ -1330,13 +1330,13 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
     // Setting to the same state shouldn't be erroneous
     stream->change_state(data.header().type, data.header().flags);
 
-    return Http2SendDataFrameResult::DONE;
+    return HTTP2_SEND_A_DATA_FRAME_DONE;
   }
 
-  return Http2SendDataFrameResult::NO_ERROR;
+  return HTTP2_SEND_A_DATA_FRAME_NO_ERROR;
 }
 
-Http2SendDataFrameResult
+void
 Http2ConnectionState::send_data_frames(Http2Stream *stream)
 {
   // To follow RFC 7540 must not send more frames other than priority on
@@ -1345,25 +1345,27 @@ Http2ConnectionState::send_data_frames(Http2Stream *stream)
       stream->get_state() == Http2StreamState::HTTP2_STREAM_STATE_CLOSED) {
     Http2StreamDebug(this->ua_session, stream->get_id(), "Shutdown half closed local stream");
     this->delete_stream(stream);
-    return Http2SendDataFrameResult::NO_ERROR;
+    return;
   }
 
-  size_t len                      = 0;
-  Http2SendDataFrameResult result = Http2SendDataFrameResult::NO_ERROR;
-  while (result == Http2SendDataFrameResult::NO_ERROR) {
-    result = send_a_data_frame(stream, len);
+  size_t len = 0;
+  while (true) {
+    Http2SendADataFrameResult result = send_a_data_frame(stream, len);
 
-    if (result == Http2SendDataFrameResult::DONE) {
+    if (result == HTTP2_SEND_A_DATA_FRAME_DONE) {
       // Delete a stream immediately
       // TODO its should not be deleted for a several time to handling
       // RST_STREAM and WINDOW_UPDATE.
       // See 'closed' state written at [RFC 7540] 5.1.
       Http2StreamDebug(this->ua_session, stream->get_id(), "Shutdown stream");
       this->delete_stream(stream);
+      break;
+    } else if (result == HTTP2_SEND_A_DATA_FRAME_NO_ERROR) {
+      continue;
+    } else {
+      break;
     }
   }
-
-  return result;
 }
 
 void
diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h
index 1a640b0..a4ffd76 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -31,11 +31,11 @@
 
 class Http2ClientSession;
 
-enum class Http2SendDataFrameResult {
-  NO_ERROR   = 0,
-  NO_WINDOW  = 1,
-  NO_PAYLOAD = 2,
-  DONE       = 3,
+enum Http2SendADataFrameResult {
+  HTTP2_SEND_A_DATA_FRAME_NO_ERROR   = 0,
+  HTTP2_SEND_A_DATA_FRAME_NO_WINDOW  = 1,
+  HTTP2_SEND_A_DATA_FRAME_NO_PAYLOAD = 2,
+  HTTP2_SEND_A_DATA_FRAME_DONE       = 3,
 };
 
 enum Http2ShutdownState { NOT_INITIATED, INITIATED, IN_PROGRESS };
@@ -218,8 +218,8 @@ public:
   // HTTP/2 frame sender
   void schedule_stream(Http2Stream *stream);
   void send_data_frames_depends_on_priority();
-  Http2SendDataFrameResult send_data_frames(Http2Stream *stream);
-  Http2SendDataFrameResult send_a_data_frame(Http2Stream *stream, size_t &payload_length);
+  void send_data_frames(Http2Stream *stream);
+  Http2SendADataFrameResult send_a_data_frame(Http2Stream *stream, size_t &payload_length);
   void send_headers_frame(Http2Stream *stream);
   void send_push_promise_frame(Http2Stream *stream, URL &url, const MIMEField *accept_encoding);
   void send_rst_stream_frame(Http2StreamId id, Http2ErrorCode ec);
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 6c08ec5..88d7ac1 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -317,32 +317,26 @@ void
 Http2Stream::do_io_close(int /* flags */)
 {
   SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
-
+  super::release(nullptr);
   if (!closed) {
-    if (parent && this->is_client_state_writeable()) {
-      // Make sure any trailing end of stream frames are sent
-      // Wee will be removed at send_data_frames or closing connection phase
-      Http2SendDataFrameResult result = static_cast<Http2ClientSession *>(parent)->connection_state.send_data_frames(this);
-      if (result == Http2SendDataFrameResult::NO_WINDOW) {
-        Http2StreamDebug("cancel closing");
-        return;
-      } else {
-        Http2StreamDebug("continue closing");
-      }
-    }
+    Http2StreamDebug("do_io_close");
 
     // When we get here, the SM has initiated the shutdown.  Either it received a WRITE_COMPLETE, or it is shutting down.  Any
     // remaining IO operations back to client should be abandoned.  The SM-side buffers backing these operations will be deleted
     // by the time this is called from transaction_done.
     closed = true;
 
+    if (parent && this->is_client_state_writeable()) {
+      // Make sure any trailing end of stream frames are sent
+      // Wee will be removed at send_data_frames or closing connection phase
+      static_cast<Http2ClientSession *>(parent)->connection_state.send_data_frames(this);
+    }
+
     clear_timers();
     clear_io_events();
 
     // Wait until transaction_done is called from HttpSM to signal that the TXN_CLOSE hook has been executed
   }
-
-  super::release(nullptr);
 }
 
 /*

-- 
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.