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.

> Maybe it should be changed to this one:
>
> if (rev->available == 0 && !rev->pending_eof) {
>
> if (rev->available <= 0 && !rev->pending_eof) {

No. Negative rev->available means we don't know how many bytes
are available, but some are. It is wrong to stop reading in
rev->available is negative.

> Also rev->available could remain negative if n != size and
> ngx_readv_chain or ngx_unix_recv wouldn't enter this blocks or if
> ngx_socket_nread failed (returns -1).

Sure, this is expected.

> And there are some code pices where nginx would expect positive
> ev->available.

If you think you know places which expect only positive
ev->available, please point them out.

> So I guess either one of this blocks are not fully correct, or perhaps
> the block [efd71d49bde0#l10.28 [6]] could be moved to end of the #if
> (NGX_HAVE_FIONREAD) block (before #endif at least in case
> !rev->pending_eof).

The code in the NGX_HAVE_FIONREAD block does not require any
particular event method and/or kernel version, so it cannot rely
on rev->pending_eof being correctly set.

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