cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] Break if is_empty_data is true at the end of the while loop

From: Tatsuhiro Tsujikawa <tatsuhiro.t_at_gmail.com>
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

Received on 2014-08-03