Skip to content

TS-4542: ASAN error with HTTP/2 #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bryancall merged 1 commit into from
Jun 29, 2016
Merged

TS-4542: ASAN error with HTTP/2 #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions proxy/http2/Http2ConnectionState.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,23 +1026,29 @@ 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<size_t>(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<size_t>(window_size));
size_t read_available_size = 0;

uint8_t flags = 0x00;
uint8_t payload_buffer[buf_len];
IOBufferReader *current_reader = stream->response_get_data_reader();

SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());

if (current_reader) {
read_available_size = static_cast<size_t>(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;
}
Expand All @@ -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;
}

Expand Down
13 changes: 10 additions & 3 deletions proxy/http2/Http2Stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ Http2Stream::do_io_close(int /* flags */)
static_cast<Http2ClientSession *>(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();
Expand All @@ -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();

Expand Down