Welcome! Log In Create A New Profile

Advanced

Re: Seg fault in http read event handler caused by rouge event call without context

Maxim Dounin
November 19, 2019 07:34AM
Hello!

On Mon, Nov 18, 2019 at 08:05:39PM +0100, Sergey Brester wrote:

> 18.11.2019 18:11, Maxim Dounin wrote:
>
> > Hello!
> >
> > On Mon, Nov 18, 2019 at 05:48:46PM +0100, Sergey Brester wrote:
> >
> >> Looks like [efd71d49bde0 [2]] could be indeed responsible for that: I see at least one state where rev->ready could remain 1 (after rev->available gets 0) e. g. deviation between blocks [efd71d49bde0#l10.8 [3]] and [efd71d49bde0#l11.8 [4]] where first did not reset rev->ready and for example if ngx_socket_nread in [efd71d49bde0#l10.38 [5]] would write 0 into rev->available, so rev->ready remains 1 yet.
> >
> > There is no deviation here. The rev->available field holds the
> > number of bytes available for reading (or -1 if it's not known),
> > while rev->ready indicates if reading is possible. The reading
> > can be possible even WHEN REV->AVAILABLE IS ZERO - notably, when
> > there is a potential pending EOF.
>
> Sure,
> but I told about the case where rev->ready indicates the READING IS
> POSSIBLE,
> but REV->AVAILABLE IS LESS AS ZERO.
>
> If you mean this is to noglect, then it is ok, but somehow it looks
> backwards incompatible to me.
> At least other async-IO procedures (for example KQUEUE [1]) handle this
> differently.

The code you are referring to defines how nginx behaves when
rev->available becomes zero or negative after it is decremented by
the amount of bytes read.

In case of kqueue, rev->available is set to 0 to indicate there
are no data to read, and rev->ready is also set to 0 unless
pending EOF is indicated by kqueue.

In case of unspecified event methods, we cannot rely on the
pending EOF indication (as this indication is only available with
kqueue and with epoll with recent enough kernels). As such, we
can only reset rev->ready when rev->available becomes negative
(and then set to 0), but not when rev->available becomes exactly
zero.

> Anyway I saw never REV->AVAILABLE LESS AS ZERO, if rev->ready got 1
> until now.

This is explained in the commit log of the revant commit. Quoting
it here:

: With other event methods rev->available is now set to -1 when the socket
: is ready for reading. Later in ngx_recv() and ngx_recv_chain(), if
: full buffer is received, real number of bytes in the socket buffer is
: retrieved using ioctl(FIONREAD). Reading more than this number of bytes
: ensures that even with edge-triggered event methods the event will be
: triggered again, so it is safe to stop processing of the socket and
: switch to other connections.

Further, it is now explicitly documented in the relevant comment in
src/event/ngx_event.h, see here:

http://hg.nginx.org/nginx/rev/efd71d49bde0#l9.1

The rev->available set to -1 now indicates that there are
unspecified amount of bytes to read.

Previously rev->available was never negative (and cannot be
negative on most platforms, given that it was a single bit
field), and certainly this is a change. And in theory it can
cause problems if some code relies on the previous semantics of
rev->available being 0 or 1 with epoll, and checks for
"rev->available == 1" for some reason (tests for "rev->available",
"!rev->available" and "rev->available == 0" will work correctly
without modifications). But in practice I wouldn't expect this
change to cause any problems with properly written code.

As already outlined in my reply to the original message, the
reason for the segmentation faults observed seems to be failure of
the upload module to block events after reading the request body.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

Seg fault in http read event handler caused by rouge event call without context

Dave Brennan 101 November 18, 2019 09:04AM

Re: Seg fault in http read event handler caused by rouge event call without context

Maxim Dounin 38 November 18, 2019 11:04AM

Re: Seg fault in http read event handler caused by rouge event call without context

Sergey Brester 39 November 18, 2019 11:50AM

Re: Seg fault in http read event handler caused by rouge event call without context

Maxim Dounin 40 November 18, 2019 12:12PM

Re: Seg fault in http read event handler caused by rouge event call without context

Sergey Brester 37 November 18, 2019 02:06PM

Re: Seg fault in http read event handler caused by rouge event call without context

Maxim Dounin 47 November 19, 2019 07:34AM

RE: Seg fault in http read event handler caused by rouge event call without context

Dave Brennan 66 November 19, 2019 09:12AM



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

Online Users

Guests: 112
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready