From d722138f29bda9386798fbd67c23a874f8992463 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 7 May 2015 17:52:48 +0200 Subject: http2: Don't call nghttp2_session_mem_recv while it is paused by a stream --- lib/http.h | 2 ++ lib/http2.c | 29 ++++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/http.h b/lib/http.h index 13fa1d99b..0b4aac343 100644 --- a/lib/http.h +++ b/lib/http.h @@ -201,6 +201,8 @@ struct http_conn { const uint8_t *upload_mem; /* points to a buffer to read from */ size_t upload_len; /* size of the buffer 'upload_mem' points to */ size_t upload_left; /* number of bytes left to upload */ + int32_t pause_stream_id; /* stream ID which paused + nghttp2_session_mem_recv */ /* this is a hash of all individual streams (SessionHandle structs) */ struct curl_hash streamsh; diff --git a/lib/http2.c b/lib/http2.c index 78a09af8e..e9eb81180 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -372,6 +372,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, stream->data = data + nread; stream->datalen = len - nread; DEBUGF(infof(data_s, "NGHTTP2_ERR_PAUSE - out of buffer\n")); + conn->proto.httpc.pause_stream_id = stream_id; return NGHTTP2_ERR_PAUSE; } return 0; @@ -762,8 +763,12 @@ CURLcode Curl_http2_request_upgrade(Curl_send_buffer *req, return result; } -static ssize_t http2_handle_stream_close(struct SessionHandle *data, +static ssize_t http2_handle_stream_close(struct http_conn *httpc, + struct SessionHandle *data, struct HTTP *stream, CURLcode *err) { + if(httpc->pause_stream_id == stream->stream_id) { + httpc->pause_stream_id = 0; + } /* Reset to FALSE to prevent infinite loop in readwrite_data function. */ stream->closed = FALSE; @@ -798,7 +803,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, otherwise, we may be going to read from underlying connection, and gets EAGAIN, and we will get stuck there. */ if(stream->memlen == 0 && stream->closed) { - return http2_handle_stream_close(data, stream, err); + return http2_handle_stream_close(httpc, data, stream, err); } /* Nullify here because we call nghttp2_session_send() and they @@ -835,6 +840,10 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, infof(data, "%zu data bytes written\n", nread); if(stream->datalen == 0) { + DEBUGF(infof(data, "Unpaused by stream %x\n", stream->stream_id)); + assert(httpc->pause_stream_id == stream->stream_id); + httpc->pause_stream_id = 0; + stream->data = NULL; stream->datalen = 0; } @@ -858,6 +867,18 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, stream->mem = mem; } } + else if(httpc->pause_stream_id) { + /* If a stream paused nghttp2_session_mem_recv previously, and has + not processed all data, it still refers to the buffer in + nghttp2_session. If we call nghttp2_session_mem_recv(), we may + overwrite that buffer. To avoid that situation, just return + here with CURLE_AGAIN. This could be busy loop since data in + socket is not read. But it seems that usually streams are + notified with its drain property, and socket is read again + quickly. */ + *err = CURLE_AGAIN; + return -1; + } else { char *inbuf; /* remember where to store incoming data for this stream and how big the @@ -939,7 +960,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, /* If stream is closed, return 0 to signal the http routine to close the connection */ if(stream->closed) { - return http2_handle_stream_close(data, stream, err); + return http2_handle_stream_close(httpc, data, stream, err); } *err = CURLE_AGAIN; DEBUGF(infof(data, "http2_recv returns -1, AGAIN\n")); @@ -1169,6 +1190,8 @@ CURLcode Curl_http2_setup(struct connectdata *conn) httpc->inbuflen = 0; httpc->nread_inbuf = 0; + httpc->pause_stream_id = 0; + conn->bits.multiplex = TRUE; /* at least potentially multiplexed */ conn->httpversion = 20; conn->bundle->server_supports_pipelining = TRUE; -- cgit v1.2.3