From 1bf2f04e3043182b65b30a5f1187d97a50335860 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Tue, 28 Jun 2016 21:18:30 -0700 Subject: [PATCH] TS-4542: ASAN error with HTTP/2 --- proxy/http2/Http2ConnectionState.cc | 24 +++++++++++++++--------- proxy/http2/Http2Stream.cc | 13 ++++++++++--- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index 6986244c41c..1bd78d417f7 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -1026,12 +1026,10 @@ Http2ConnectionState::send_data_frames_depends_on_priority() Http2SendADataFrameResult Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_length) { - const ssize_t window_size = min(this->client_rwnd, stream->client_rwnd); - if (window_size <= 0) { - return HTTP2_SEND_A_DATA_FRAME_NO_WINDOW; - } - const size_t buf_len = BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_DATA]) - HTTP2_FRAME_HEADER_LEN; - const size_t available_size = min(buf_len, static_cast(window_size)); + const ssize_t window_size = min(this->client_rwnd, stream->client_rwnd); + const size_t buf_len = BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_DATA]) - HTTP2_FRAME_HEADER_LEN; + const size_t write_available_size = min(buf_len, static_cast(window_size)); + size_t read_available_size = 0; uint8_t flags = 0x00; uint8_t payload_buffer[buf_len]; @@ -1039,10 +1037,18 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread()); + if (current_reader) { + read_available_size = static_cast(current_reader->read_avail()); + } + // Select appropriate payload length - if (current_reader && current_reader->is_read_avail_more_than(0)) { + if (read_available_size > 0) { + // We only need to check for window size when there is a payload + if (window_size <= 0) { + 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, available_size); + payload_length = current_reader->read(payload_buffer, write_available_size); } else { payload_length = 0; } @@ -1054,7 +1060,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len return HTTP2_SEND_A_DATA_FRAME_NO_PAYLOAD; } - if (stream->is_body_done() && payload_length < available_size) { + if (stream->is_body_done() && read_available_size <= write_available_size) { flags |= HTTP2_FLAGS_DATA_END_STREAM; } diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc index 482898acc7e..f373a2d02a3 100644 --- a/proxy/http2/Http2Stream.cc +++ b/proxy/http2/Http2Stream.cc @@ -269,6 +269,8 @@ Http2Stream::do_io_close(int /* flags */) static_cast(parent)->connection_state.send_data_frames(this); } parent = NULL; + // Check to see if the stream is in the closed state + ink_assert(get_state() == HTTP2_STREAM_STATE_CLOSED); clear_timers(); clear_io_events(); @@ -291,12 +293,17 @@ Http2Stream::initiating_close() if (!closed) { SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread()); Debug("http2_stream", "initiating_close stream %d", this->get_id()); + + // Set the state of the connection to closed + // TODO - these states should be combined closed = true; - // leaving the reference to the SM, so we can detatch from the SM - // when we actually destroy - // current_reader = NULL; + _state = HTTP2_STREAM_STATE_CLOSED; parent = NULL; + + // leaving the reference to the SM, so we can detatch from the SM when we actually destroy + // current_reader = NULL; + clear_timers(); clear_io_events();