curl-library
Re: [PATCH] Break if is_empty_data is true at the end of the while loop
Date: Sun, 3 Aug 2014 14:05:10 +0900
On Sun, Aug 3, 2014 at 6:37 AM, Daniel Stenberg <daniel_at_haxx.se> wrote:
> On Sat, 2 Aug 2014, Tatsuhiro Tsujikawa wrote:
>
> This patch adds break statement if is_empty_data is true at the end of
>> while loop in readwrite_data. Without this break statement, CPU usage goes
>> to 100% if HTTP/2 transfer is interrupted (RST_STREAM) and its recv
>> function keeps return 0.
>>
>
> I don't think it is the right change. I'm worried we have cases where
> there's still data to send in when the end of reading condition occurs.
>
> We have a funky condition for nghttp2 i the data_pending() function we
> should probably tweak instead so that it won't say that there is data
> pending when we know there's not!
>
> How did you test this? I'm thinking perhaps another fix is to clear
> httpc->closed after it has served its purpose once, like:
>
>
To test this, stream must be reset (RST_STREAM) before any response.
There is no option to cause this issue in nghttp2 servers.
Easiest way is add
```
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
```
at the front of on_header_callback in shrpx_http2_upstream.cc and test curl
with nghttpx.
Your patch works fine for me. Patch attached based on yours.
Best regards,
Tatsuhiro Tsujikawa
> --- a/lib/http2.c
> +++ b/lib/http2.c
> @@ -666,10 +666,11 @@ static ssize_t http2_recv(struct connectdata *conn,
> int socki
> struct http_conn *httpc = &conn->proto.httpc;
>
> (void)sockindex; /* we always do HTTP2 on sockindex 0 */
>
> if(httpc->closed) {
> + httpc->closed = FALSE;
> return 0;
> }
>
> /* Nullify here because we call nghttp2_session_send() and they
> might refer to the old buffer. */
> @@ -745,10 +746,11 @@ static ssize_t http2_recv(struct connectdata *conn,
> int socki
> return len - conn->proto.httpc.len;
> }
> /* If stream is closed, return 0 to signal the http routine to close
> the connection */
> if(httpc->closed) {
> + httpc->closed = FALSE;
> return 0;
> }
> *err = CURLE_AGAIN;
> return -1;
> }
>
>
> --
>
> / daniel.haxx.se
> -------------------------------------------------------------------
> List admin: http://cool.haxx.se/list/listinfo/curl-library
> Etiquette: http://curl.haxx.se/mail/etiquette.html
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
- application/x-gzip attachment: 0001-HTTP-2-Fix-infinite-loop-in-readwrite_data-when-stre.patch.gz