<feed xmlns='http://www.w3.org/2005/Atom'>
<title>nginx.git/src/event/ngx_event_pipe.c, branch release-1.29.2</title>
<subtitle>nginx</subtitle>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/'/>
<entry>
<title>Upstream: fixed usage of closed sockets with filter finalization.</title>
<updated>2024-01-30T00:20:10+00:00</updated>
<author>
<name>Maxim Dounin</name>
<email>mdounin@mdounin.ru</email>
</author>
<published>2024-01-30T00:20:10+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=6f2059147f20d1bd2cd6ff01ea71bf31ec9c2845'/>
<id>6f2059147f20d1bd2cd6ff01ea71bf31ec9c2845</id>
<content type='text'>
When filter finalization is triggered when working with an upstream server,
and error_page redirects request processing to some simple handler,
ngx_http_request_finalize() triggers request termination when the response
is sent.  In particular, via the upstream cleanup handler, nginx will close
the upstream connection and the corresponding socket.

Still, this can happen to be with ngx_event_pipe() on stack.  While
the code will set p-&gt;downstream_error due to NGX_ERROR returned from the
output filter chain by filter finalization, otherwise the error will be
ignored till control returns to ngx_http_upstream_process_request().
And event pipe might try reading from the (already closed) socket, resulting
in "readv() failed (9: Bad file descriptor) while reading upstream" errors
(or even segfaults with SSL).

Such errors were seen with the following configuration:

    location /t2 {
        proxy_pass http://127.0.0.1:8080/big;

        image_filter_buffer 10m;
        image_filter   resize  150 100;
        error_page     415   = /empty;
    }

    location /empty {
        return 204;
    }

    location /big {
        # big enough static file
    }

Fix is to clear p-&gt;upstream in ngx_http_upstream_finalize_request(),
and ensure that p-&gt;upstream is checked in ngx_event_pipe_read_upstream()
and when handling events at ngx_event_pipe() exit.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When filter finalization is triggered when working with an upstream server,
and error_page redirects request processing to some simple handler,
ngx_http_request_finalize() triggers request termination when the response
is sent.  In particular, via the upstream cleanup handler, nginx will close
the upstream connection and the corresponding socket.

Still, this can happen to be with ngx_event_pipe() on stack.  While
the code will set p-&gt;downstream_error due to NGX_ERROR returned from the
output filter chain by filter finalization, otherwise the error will be
ignored till control returns to ngx_http_upstream_process_request().
And event pipe might try reading from the (already closed) socket, resulting
in "readv() failed (9: Bad file descriptor) while reading upstream" errors
(or even segfaults with SSL).

Such errors were seen with the following configuration:

    location /t2 {
        proxy_pass http://127.0.0.1:8080/big;

        image_filter_buffer 10m;
        image_filter   resize  150 100;
        error_page     415   = /empty;
    }

    location /empty {
        return 204;
    }

    location /big {
        # big enough static file
    }

Fix is to clear p-&gt;upstream in ngx_http_upstream_finalize_request(),
and ensure that p-&gt;upstream is checked in ngx_event_pipe_read_upstream()
and when handling events at ngx_event_pipe() exit.
</pre>
</div>
</content>
</entry>
<entry>
<title>Upstream: drop extra data sent by upstream.</title>
<updated>2020-07-06T15:36:22+00:00</updated>
<author>
<name>Maxim Dounin</name>
<email>mdounin@mdounin.ru</email>
</author>
<published>2020-07-06T15:36:22+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=dfcfcc5a881bf4b349f74c9a0a04da2d861f02bf'/>
<id>dfcfcc5a881bf4b349f74c9a0a04da2d861f02bf</id>
<content type='text'>
Previous behaviour was to pass everything to the client, but this
seems to be suboptimal and causes issues (ticket #1695).  Fix is to
drop extra data instead, as it naturally happens in most clients.

This change covers generic buffered and unbuffered filters as used
in the scgi and uwsgi modules.  Appropriate input filter init
handlers are provided by the scgi and uwsgi modules to set corresponding
lengths.

Note that for responses to HEAD requests there is an exception:
we do allow any response length.  This is because responses to HEAD
requests might be actual full responses, and it is up to nginx
to remove the response body.  If caching is enabled, only full
responses matching the Content-Length header will be cached
(see b779728b180c).
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Previous behaviour was to pass everything to the client, but this
seems to be suboptimal and causes issues (ticket #1695).  Fix is to
drop extra data instead, as it naturally happens in most clients.

This change covers generic buffered and unbuffered filters as used
in the scgi and uwsgi modules.  Appropriate input filter init
handlers are provided by the scgi and uwsgi modules to set corresponding
lengths.

Note that for responses to HEAD requests there is an exception:
we do allow any response length.  This is because responses to HEAD
requests might be actual full responses, and it is up to nginx
to remove the response body.  If caching is enabled, only full
responses matching the Content-Length header will be cached
(see b779728b180c).
</pre>
</div>
</content>
</entry>
<entry>
<title>Event pipe: disabled c-&gt;read-&gt;available checking for SSL.</title>
<updated>2019-10-17T13:02:03+00:00</updated>
<author>
<name>Maxim Dounin</name>
<email>mdounin@mdounin.ru</email>
</author>
<published>2019-10-17T13:02:03+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=60609f2372f62628191bf01ed856a46cd488921b'/>
<id>60609f2372f62628191bf01ed856a46cd488921b</id>
<content type='text'>
In SSL connections, data can be buffered by the SSL layer, and it is
wrong to avoid doing c-&gt;recv_chain() if c-&gt;read-&gt;available is 0 and
c-&gt;read-&gt;pending_eof is set.  And tests show that the optimization in
question indeed can result in incorrect detection of premature connection
close if upstream closes the connection without sending a close notify
alert at the same time.  Fix is to disable c-&gt;read-&gt;available optimization
for SSL connections.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
In SSL connections, data can be buffered by the SSL layer, and it is
wrong to avoid doing c-&gt;recv_chain() if c-&gt;read-&gt;available is 0 and
c-&gt;read-&gt;pending_eof is set.  And tests show that the optimization in
question indeed can result in incorrect detection of premature connection
close if upstream closes the connection without sending a close notify
alert at the same time.  Fix is to disable c-&gt;read-&gt;available optimization
for SSL connections.
</pre>
</div>
</content>
</entry>
<entry>
<title>Upstream: fixed cache corruption and socket leaks with aio_write.</title>
<updated>2017-01-20T18:14:19+00:00</updated>
<author>
<name>Maxim Dounin</name>
<email>mdounin@mdounin.ru</email>
</author>
<published>2017-01-20T18:14:19+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=e66073c4d3a3d25b001dee617ee22d2a969ceab2'/>
<id>e66073c4d3a3d25b001dee617ee22d2a969ceab2</id>
<content type='text'>
The ngx_event_pipe() function wasn't called on write events with
wev-&gt;delayed set.  As a result, threaded writing results weren't
properly collected in ngx_event_pipe_write_to_downstream() when a
write event was triggered for a completed write.

Further, this wasn't detected, as p-&gt;aio was reset by a thread completion
handler, and results were later collected in ngx_event_pipe_read_upstream()
instead of scheduling a new write of additional data.  If this happened
on the last reading from an upstream, last part of the response was never
written to the cache file.

Similar problems might also happen in case of timeouts when writing to
client, as this also results in ngx_event_pipe() not being called on write
events.  In this scenario socket leaks were observed.

Fix is to check if p-&gt;writing is set in ngx_event_pipe_read_upstream(), and
therefore collect results of previous write operations in case of read events
as well, similar to how we do so in ngx_event_pipe_write_downstream().
This is enough to fix the wev-&gt;delayed case.  Additionally, we now call
ngx_event_pipe() from ngx_http_upstream_process_request() if there are
uncollected write operations (p-&gt;writing and !p-&gt;aio).  This also fixes
the wev-&gt;timedout case.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The ngx_event_pipe() function wasn't called on write events with
wev-&gt;delayed set.  As a result, threaded writing results weren't
properly collected in ngx_event_pipe_write_to_downstream() when a
write event was triggered for a completed write.

Further, this wasn't detected, as p-&gt;aio was reset by a thread completion
handler, and results were later collected in ngx_event_pipe_read_upstream()
instead of scheduling a new write of additional data.  If this happened
on the last reading from an upstream, last part of the response was never
written to the cache file.

Similar problems might also happen in case of timeouts when writing to
client, as this also results in ngx_event_pipe() not being called on write
events.  In this scenario socket leaks were observed.

Fix is to check if p-&gt;writing is set in ngx_event_pipe_read_upstream(), and
therefore collect results of previous write operations in case of read events
as well, similar to how we do so in ngx_event_pipe_write_downstream().
This is enough to fix the wev-&gt;delayed case.  Additionally, we now call
ngx_event_pipe() from ngx_http_upstream_process_request() if there are
uncollected write operations (p-&gt;writing and !p-&gt;aio).  This also fixes
the wev-&gt;timedout case.
</pre>
</div>
</content>
</entry>
<entry>
<title>Event pipe: do not set file's thread_handler if not needed.</title>
<updated>2016-09-01T17:05:23+00:00</updated>
<author>
<name>Maxim Dounin</name>
<email>mdounin@mdounin.ru</email>
</author>
<published>2016-09-01T17:05:23+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=ea9a1d745b5db26cac2d4b6e231ca6c4af0b4e5a'/>
<id>ea9a1d745b5db26cac2d4b6e231ca6c4af0b4e5a</id>
<content type='text'>
This fixes a problem with aio threads and sendfile with aio_write switched
off, as observed with range requests after fc72784b1f52 (1.9.13).  Potential
problems with sendfile in threads were previously described in 9fd738b85fad,
and this seems to be one of them.

The problem occurred as file's thread_handler was set to NULL by event pipe
code after a sendfile thread task was scheduled.  As a result, no sendfile
completion code was executed, and the same buffer was additionally sent
using non-threaded sendfile.  Fix is to avoid modifying file's thread_handler
if aio_write is switched off.

Note that with "aio_write on" it is still possible that sendfile will use
thread_handler as set by event pipe.  This is believed to be safe though,
as handlers used are compatible.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This fixes a problem with aio threads and sendfile with aio_write switched
off, as observed with range requests after fc72784b1f52 (1.9.13).  Potential
problems with sendfile in threads were previously described in 9fd738b85fad,
and this seems to be one of them.

The problem occurred as file's thread_handler was set to NULL by event pipe
code after a sendfile thread task was scheduled.  As a result, no sendfile
completion code was executed, and the same buffer was additionally sent
using non-threaded sendfile.  Fix is to avoid modifying file's thread_handler
if aio_write is switched off.

Note that with "aio_write on" it is still possible that sendfile will use
thread_handler as set by event pipe.  This is believed to be safe though,
as handlers used are compatible.
</pre>
</div>
</content>
</entry>
<entry>
<title>Event pipe: process data after recv_chain() errors.</title>
<updated>2016-09-01T15:29:55+00:00</updated>
<author>
<name>Maxim Dounin</name>
<email>mdounin@mdounin.ru</email>
</author>
<published>2016-09-01T15:29:55+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=4e8a73adaa7b5bddeba01daf1965d66a897f066d'/>
<id>4e8a73adaa7b5bddeba01daf1965d66a897f066d</id>
<content type='text'>
When c-&gt;recv_chain() returns an error, it is possible that we already
have some data previously read, e.g., in preread buffer.  And in some
cases it may be even a complete response.  Changed c-&gt;recv_chain() error
handling to process the data, much like it is already done if kevent
reports about an error.

This change, in particular, fixes processing of small responses
when an upstream fails to properly close a connection with lingering and
therefore the connection is reset, but the response is already fully
obtained by nginx (see ticket #1037).
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When c-&gt;recv_chain() returns an error, it is possible that we already
have some data previously read, e.g., in preread buffer.  And in some
cases it may be even a complete response.  Changed c-&gt;recv_chain() error
handling to process the data, much like it is already done if kevent
reports about an error.

This change, in particular, fixes processing of small responses
when an upstream fails to properly close a connection with lingering and
therefore the connection is reset, but the response is already fully
obtained by nginx (see ticket #1037).
</pre>
</div>
</content>
</entry>
<entry>
<title>Fixed logging.</title>
<updated>2016-03-30T23:33:57+00:00</updated>
<author>
<name>Sergey Kandaurov</name>
<email>pluknet@nginx.com</email>
</author>
<published>2016-03-30T23:33:57+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=00ef9ff5f03ce7e98ba64c3644da25e5a0d659fc'/>
<id>00ef9ff5f03ce7e98ba64c3644da25e5a0d659fc</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Style.</title>
<updated>2016-03-30T08:52:16+00:00</updated>
<author>
<name>Ruslan Ermilov</name>
<email>ru@nginx.com</email>
</author>
<published>2016-03-30T08:52:16+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=7ad57da59821294255610545b2b5ce07e74124a5'/>
<id>7ad57da59821294255610545b2b5ce07e74124a5</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Threads: writing via threads pools in event pipe.</title>
<updated>2016-03-18T03:44:49+00:00</updated>
<author>
<name>Maxim Dounin</name>
<email>mdounin@mdounin.ru</email>
</author>
<published>2016-03-18T03:44:49+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=348f705c000bdbfbee74d6f0111a03697f8ffa4f'/>
<id>348f705c000bdbfbee74d6f0111a03697f8ffa4f</id>
<content type='text'>
The "aio_write" directive is introduced, which enables use of aio
for writing.  Currently it is meaningful only with "aio threads".

Note that aio operations can be done by both event pipe and output
chain, so proper mapping between r-&gt;aio and p-&gt;aio is provided when
calling ngx_event_pipe() and in output filter.

In collaboration with Valentin Bartenev.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The "aio_write" directive is introduced, which enables use of aio
for writing.  Currently it is meaningful only with "aio threads".

Note that aio operations can be done by both event pipe and output
chain, so proper mapping between r-&gt;aio and p-&gt;aio is provided when
calling ngx_event_pipe() and in output filter.

In collaboration with Valentin Bartenev.
</pre>
</div>
</content>
</entry>
<entry>
<title>Event pipe: call ngx_handle_read_event() with a proper flags type.</title>
<updated>2015-06-03T16:12:26+00:00</updated>
<author>
<name>Sergey Kandaurov</name>
<email>pluknet@nginx.com</email>
</author>
<published>2015-06-03T16:12:26+00:00</published>
<link rel='alternate' type='text/html' href='https://git.sigsegv.uk/nginx.git/commit/?id=99b2c89404d6ee06a6df0645a676845c22f80ea8'/>
<id>99b2c89404d6ee06a6df0645a676845c22f80ea8</id>
<content type='text'>
The change was missed in f69d1aab6a0f.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The change was missed in f69d1aab6a0f.
</pre>
</div>
</content>
</entry>
</feed>
