<div dir="ltr">Hi Maxim,<div><br></div><div>it is pretty hard to reproduce reliably full socket buffer in a test. I'll play with UNIX domain sockets, maybe I'll manage to reproduce it there since you have somewhat better control over it.</div><div><br></div><div>Thanks,</div><div>Patryk<br><br><div class="gmail_quote"><div dir="ltr">On Tue, Dec 5, 2017 at 7:45 AM Maxim Konovalov <<a href="mailto:maxim@nginx.com">maxim@nginx.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 05/12/2017 18:16, Maxim Dounin wrote:<br>
> Hello!<br>
><br>
> On Fri, Dec 01, 2017 at 04:49:29PM -0800, Piotr Sikora via nginx-devel wrote:<br>
><br>
>> # HG changeset patch<br>
>> # User Patryk Lesiewicz <<a href="mailto:patryk@google.com" target="_blank">patryk@google.com</a>><br>
>> # Date 1512172754 28800<br>
>> #      Fri Dec 01 15:59:14 2017 -0800<br>
>> # Node ID 0e2e2da798261fe5105017d9678566267b07e2b9<br>
>> # Parent  fc0d06224edac2c7cfbfd9a4def478f285d9957b<br>
>> Upstream: flush low-level buffers on write retry.<br>
>><br>
>> If the data to write is bigger than what the socket can send, and the<br>
>> reminder is smaller than NGX_SSL_BUFSIZE, then SSL_write() fails with<br>
>> SSL_ERROR_WANT_WRITE. The reminder of payload however is successfully<br>
>> copied to the low-level buffer and all the output chain buffers are<br>
>> flushed. This means that retry logic doesn't work because<br>
>> ngx_http_upstream_process_non_buffered_request() checks only if there's<br>
>> anything in the output chain buffers and ignores the fact that something<br>
>> may be buffered in low-level parts of the stack.<br>
>><br>
>> Signed-off-by: Patryk Lesiewicz <<a href="mailto:patryk@google.com" target="_blank">patryk@google.com</a>><br>
>><br>
>> diff -r fc0d06224eda -r 0e2e2da79826 src/http/ngx_http_upstream.c<br>
>> --- a/src/http/ngx_http_upstream.c<br>
>> +++ b/src/http/ngx_http_upstream.c<br>
>> @@ -3533,7 +3533,7 @@ ngx_http_upstream_process_non_buffered_r<br>
>><br>
>>          if (do_write) {<br>
>><br>
>> -            if (u->out_bufs || u->busy_bufs) {<br>
>> +            if (u->out_bufs || u->busy_bufs || downstream->buffered) {<br>
>>                  rc = ngx_http_output_filter(r, u->out_bufs);<br>
>><br>
>>                  if (rc == NGX_ERROR) {<br>
><br>
> Committed, thanks for catching this.<br>
> <a href="http://hg.nginx.org/nginx/rev/a762ddf22dbb" rel="noreferrer" target="_blank">http://hg.nginx.org/nginx/rev/a762ddf22dbb</a><br>
><br>
Do we need tests for this scenario?<br>
<br>
--<br>
Maxim Konovalov<br>
<br>
"I'm not a software developer, but it doesn't seem as rocket<br>
science"<br>
</blockquote></div></div></div>