Patryk Lesiewicz via nginx-devel
December 05, 2017 06:46PM
Hi Maxim,

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.

Thanks,
Patryk

On Tue, Dec 5, 2017 at 7:45 AM Maxim Konovalov <maxim@nginx.com> wrote:

> On 05/12/2017 18:16, Maxim Dounin wrote:
> > Hello!
> >
> > On Fri, Dec 01, 2017 at 04:49:29PM -0800, Piotr Sikora via nginx-devel
> wrote:
> >
> >> # HG changeset patch
> >> # User Patryk Lesiewicz <patryk@google.com>
> >> # Date 1512172754 28800
> >> # Fri Dec 01 15:59:14 2017 -0800
> >> # Node ID 0e2e2da798261fe5105017d9678566267b07e2b9
> >> # Parent fc0d06224edac2c7cfbfd9a4def478f285d9957b
> >> Upstream: flush low-level buffers on write retry.
> >>
> >> If the data to write is bigger than what the socket can send, and the
> >> reminder is smaller than NGX_SSL_BUFSIZE, then SSL_write() fails with
> >> SSL_ERROR_WANT_WRITE. The reminder of payload however is successfully
> >> copied to the low-level buffer and all the output chain buffers are
> >> flushed. This means that retry logic doesn't work because
> >> ngx_http_upstream_process_non_buffered_request() checks only if there's
> >> anything in the output chain buffers and ignores the fact that something
> >> may be buffered in low-level parts of the stack.
> >>
> >> Signed-off-by: Patryk Lesiewicz <patryk@google.com>
> >>
> >> diff -r fc0d06224eda -r 0e2e2da79826 src/http/ngx_http_upstream.c
> >> --- a/src/http/ngx_http_upstream.c
> >> +++ b/src/http/ngx_http_upstream.c
> >> @@ -3533,7 +3533,7 @@ ngx_http_upstream_process_non_buffered_r
> >>
> >> if (do_write) {
> >>
> >> - if (u->out_bufs || u->busy_bufs) {
> >> + if (u->out_bufs || u->busy_bufs || downstream->buffered) {
> >> rc = ngx_http_output_filter(r, u->out_bufs);
> >>
> >> if (rc == NGX_ERROR) {
> >
> > Committed, thanks for catching this.
> > http://hg.nginx.org/nginx/rev/a762ddf22dbb
> >
> Do we need tests for this scenario?
>
> --
> Maxim Konovalov
>
> "I'm not a software developer, but it doesn't seem as rocket
> science"
>
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] Upstream: flush low-level buffers on write retry

Piotr Sikora via nginx-devel 418 December 01, 2017 07:52PM

Re: [PATCH] Upstream: flush low-level buffers on write retry

Maxim Dounin 150 December 05, 2017 10:18AM

Re: [PATCH] Upstream: flush low-level buffers on write retry

Maxim Konovalov 138 December 05, 2017 10:46AM

Re: [PATCH] Upstream: flush low-level buffers on write retry

Patryk Lesiewicz via nginx-devel 188 December 05, 2017 06:46PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 303
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready