You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by so...@apache.org on 2017/02/10 16:53:55 UTC
[trafficserver] branch 6.2.x updated: TS-4665: Http2 not
terminating stream with short chunked response.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/6.2.x by this push:
new 46794f8 TS-4665: Http2 not terminating stream with short chunked response.
46794f8 is described below
commit 46794f84f4153c91f46b1f4f31bac0a58d2d97af
Author: Susan Hinrichs <sh...@ieee.org>
AuthorDate: Mon Aug 22 14:33:57 2016 +0000
TS-4665: Http2 not terminating stream with short chunked response.
(cherry picked from commit 5e9b18a1a8ed584e4e7cbbab9121b303b9465915)
---
proxy/http2/Http2Stream.cc | 49 ++++++++++++++++++++++++++++++----------------
proxy/http2/Http2Stream.h | 5 +++++
2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 5794bef..2b8ef80 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -110,12 +110,23 @@ Http2Stream::main_event_handler(int event, void *edata)
}
break;
case VC_EVENT_EOS: {
- SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
- // Clean up after yourself if this was an EOS
- ink_release_assert(this->closed);
- // Safe to initiate SSN_CLOSE if this is the last stream
- static_cast<Http2ClientSession *>(parent)->connection_state.release_stream(this);
- this->destroy();
+ // If there are active VIO's send the EOS through them
+ if (e->cookie == &read_vio) {
+ SCOPED_MUTEX_LOCK(lock, read_vio.mutex, this_ethread());
+ read_vio._cont->handleEvent(VC_EVENT_EOS, &read_vio);
+ } else if (e->cookie == &write_vio) {
+ SCOPED_MUTEX_LOCK(lock, write_vio.mutex, this_ethread());
+ write_vio._cont->handleEvent(VC_EVENT_EOS, &write_vio);
+ } else {
+ // Otherwise, handle the EOS yourself and shut down
+ SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
+ // Clean up after yourself if this was an EOS
+ ink_release_assert(this->closed);
+
+ // Safe to initiate SSN_CLOSE if this is the last stream
+ static_cast<Http2ClientSession *>(parent)->connection_state.release_stream(this);
+ this->destroy();
+ }
break;
}
}
@@ -289,7 +300,7 @@ Http2Stream::do_io_close(int /* flags */)
sent_delete = true;
closed = true;
- if (parent) {
+ 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);
@@ -350,18 +361,20 @@ Http2Stream::initiating_close()
// This should result in do_io_close or release being called. That will schedule the final
// kill yourself signal
- // Send the SM the EOS signal
+ // Send the SM the EOS signal if there are no active VIO's to signal
+ // We are sending signals rather than calling the handlers directly to avoid the case where
+ // the HttpTunnel handler causes the HttpSM to be deleted on the stack.
bool sent_write_complete = false;
- if (current_reader) {
+ if (current_reader && this->is_client_state_writeable()) {
// Push out any last IO events
if (write_vio._cont) {
SCOPED_MUTEX_LOCK(lock, write_vio.mutex, this_ethread());
// Are we done?
if (write_vio.nbytes == write_vio.ndone) {
Debug("http2_stream", "handle write from destroy stream=%d event=%d", this->_id, VC_EVENT_WRITE_COMPLETE);
- write_vio._cont->handleEvent(VC_EVENT_WRITE_COMPLETE, &write_vio);
+ write_event = send_tracked_event(write_event, VC_EVENT_WRITE_COMPLETE, &write_vio);
} else {
- write_vio._cont->handleEvent(VC_EVENT_EOS, &write_vio);
+ write_event = send_tracked_event(write_event, VC_EVENT_EOS, &write_vio);
Debug("http2_stream", "handle write from destroy stream=%d event=%d", this->_id, VC_EVENT_EOS);
}
sent_write_complete = true;
@@ -373,7 +386,7 @@ Http2Stream::initiating_close()
if (!sent_write_complete) {
SCOPED_MUTEX_LOCK(lock, read_vio.mutex, this_ethread());
Debug("http2_stream", "send EOS to read cont stream=%d", this->_id);
- read_vio._cont->handleEvent(VC_EVENT_EOS, &read_vio);
+ read_event = send_tracked_event(read_event, VC_EVENT_EOS, &read_vio);
}
} else if (current_reader) {
SCOPED_MUTEX_LOCK(lock, current_reader->mutex, this_ethread());
@@ -461,9 +474,6 @@ bool
Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool call_update)
{
bool retval = true;
- if (closed || sent_delete || parent == NULL) {
- return retval;
- }
if (this->get_thread() != this_ethread()) {
SCOPED_MUTEX_LOCK(stream_lock, this->mutex, this_ethread());
if (cross_thread_event == NULL) {
@@ -477,6 +487,9 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
// Copy over data in the abuffer into resp_buffer. Then schedule a WRITE_READY or
// WRITE_COMPLETE event
SCOPED_MUTEX_LOCK(lock, write_vio.mutex, this_ethread());
+ if (!this->is_client_state_writeable() || closed || sent_delete || parent == NULL) {
+ return retval;
+ }
int64_t total_added = 0;
if (write_vio.nbytes > 0 && write_vio.ndone < write_vio.nbytes) {
int64_t num_to_write = write_vio.nbytes - write_vio.ndone;
@@ -512,6 +525,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
parent->connection_state.send_headers_frame(this);
// See if the response is chunked. Set up the dechunking logic if it is
+ // Make sure to check if the chunk is complete and signal appropriately
this->response_initialize_data_handling(is_done);
if (is_done) {
send_event = VC_EVENT_WRITE_COMPLETE;
@@ -521,6 +535,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
// make sure to send the end of stream
if (this->response_is_data_available() || send_event == VC_EVENT_WRITE_COMPLETE) {
if (send_event != VC_EVENT_WRITE_COMPLETE) {
+ send_response_body();
// As with update_read_request, should be safe to call handler directly here if
// call_update is true. Commented out for now while tracking a performance regression
if (call_update) { // Coming from reenable. Safe to call the handler directly
@@ -532,10 +547,10 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
}
} else {
this->mark_body_done();
+ // Send the data frame
+ send_response_body();
retval = false;
}
- // Send the data frame
- send_response_body();
}
break;
}
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 2f9ecd0..468bf46 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -223,6 +223,11 @@ public:
void clear_active_timer();
void clear_timers();
void clear_io_events();
+ bool
+ is_client_state_writeable()
+ {
+ return _state == HTTP2_STREAM_STATE_OPEN || _state == HTTP2_STREAM_STATE_HALF_CLOSED_REMOTE;
+ }
private:
void response_initialize_data_handling(bool &is_done);
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].