Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Graham Leggett]

On 22 Mar 2016, at 7:11 PM, Yann Ylavic  wrote:

> In r1736216, I tried to restore c->data_in_input_filters usage as
> prior to r1734656.
> Does it help (or I'll revert)?

Got caught up in other stuff before I could get to the bottom of this :(

It looks like we need to restore c->data_in_input_filters usage.

What this c->data_in_input_filters does is do a poor man’s compensation for 
input filters that buffer data during this particular phase of the request 
handling without having a mechanism to tell anybody. Without telling us they’ve 
buffered data, we’ve got to work out ourselves whether we should loop round and 
poll for read, or not. If we make a mistake, we poll for a read that was 
already read, and we hang.

This is really hit and miss - if you have input filters that buffer data in the 
server you’ll hang, but if you don’t have input filters that buffer data in the 
server you don’t hang and the test suite works fine.

When v2.6 comes we must remove c-> data_in_input_filters and use the proper 
mechanism for input filters to signal they have buffered data.

Regards,
Graham
—

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Yann Ylavic]

In r1736216, I tried to restore c->data_in_input_filters usage as
prior to r1734656.
Does it help (or I'll revert)?


On Tue, Mar 22, 2016 at 1:02 PM, Jim Jagielski  wrote:
> Well, we gotta do something. This is a significant breakage.
>
>> On Mar 15, 2016, at 4:29 AM, Yann Ylavic  wrote:
>>
>> On Tue, Mar 15, 2016 at 12:42 AM, Graham Leggett  wrote:
>>> On 14 Mar 2016, at 10:48 AM, Ruediger Pluem  wrote:
>>>
 This seems to cause frequent (no always) failures with test 8 of 
 t/ssl/proxy.t.
 The request times out with a 504 status. So it looks like the "backend" in 
 this request does not respond.
 Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
 closeset guess would be that the changes to
 checkpipeline cause this.
>>>
>>> I commented out check_pipeline() completely, and it passed all the tests.
>>>
>>> Looking at check_pipeline, it seems to try and eat trailing CRLFs, which is 
>>> duplicating the functionality in read_request_line() which eats preceding 
>>> CRLFs already.
>>>
>>> check_pipeline() seems to have been overhauled recently, not sure what 
>>> problem it is trying to solve. Can we remove it?
>>
>> No we can't, or at least we need to eat the trailing CRLFs.
>> Otherwise they will be considered as a pipelined request, thus if no
>> request is really following, we will block in read_request_line(), w/o
>> FLUSHing the previous response.
>> For mpm event, that also causes a spurious wake up (defeating
>> non-blocking since read_request_line() is/was? blocking).
>>
>> But maybe your recent changes could avoid this, dunno, at least we
>> need to flush when there is no real pipelined request already there
>> (after the CRLFs).
>>
>> Regards,
>> Yann.
>

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Jim Jagielski]

Well, we gotta do something. This is a significant breakage.

> On Mar 15, 2016, at 4:29 AM, Yann Ylavic  wrote:
> 
> On Tue, Mar 15, 2016 at 12:42 AM, Graham Leggett  wrote:
>> On 14 Mar 2016, at 10:48 AM, Ruediger Pluem  wrote:
>> 
>>> This seems to cause frequent (no always) failures with test 8 of 
>>> t/ssl/proxy.t.
>>> The request times out with a 504 status. So it looks like the "backend" in 
>>> this request does not respond.
>>> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
>>> closeset guess would be that the changes to
>>> checkpipeline cause this.
>> 
>> I commented out check_pipeline() completely, and it passed all the tests.
>> 
>> Looking at check_pipeline, it seems to try and eat trailing CRLFs, which is 
>> duplicating the functionality in read_request_line() which eats preceding 
>> CRLFs already.
>> 
>> check_pipeline() seems to have been overhauled recently, not sure what 
>> problem it is trying to solve. Can we remove it?
> 
> No we can't, or at least we need to eat the trailing CRLFs.
> Otherwise they will be considered as a pipelined request, thus if no
> request is really following, we will block in read_request_line(), w/o
> FLUSHing the previous response.
> For mpm event, that also causes a spurious wake up (defeating
> non-blocking since read_request_line() is/was? blocking).
> 
> But maybe your recent changes could avoid this, dunno, at least we
> need to flush when there is no real pipelined request already there
> (after the CRLFs).
> 
> Regards,
> Yann.

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Rainer Jung]

Am 14.03.2016 um 09:48 schrieb Ruediger Pluem:

On 03/12/2016 01:43 AM, minf...@apache.org wrote:

Author: minfrin
Date: Sat Mar 12 00:43:58 2016
New Revision: 1734656

URL: http://svn.apache.org/viewvc?rev=1734656=rev
Log:
core: Extend support for setting aside data from the network input filter
to any connection or request input filter.

Modified:
 httpd/httpd/trunk/CHANGES
 httpd/httpd/trunk/include/ap_mmn.h
 httpd/httpd/trunk/include/httpd.h
 httpd/httpd/trunk/include/mpm_common.h
 httpd/httpd/trunk/include/util_filter.h
 httpd/httpd/trunk/modules/http/http_core.c
 httpd/httpd/trunk/modules/http/http_request.c
 httpd/httpd/trunk/server/core.c
 httpd/httpd/trunk/server/core_filters.c
 httpd/httpd/trunk/server/mpm/event/event.c
 httpd/httpd/trunk/server/mpm/motorz/motorz.c
 httpd/httpd/trunk/server/mpm/simple/simple_io.c
 httpd/httpd/trunk/server/mpm_common.c
 httpd/httpd/trunk/server/util_filter.c




This seems to cause frequent (no always) failures with test 8 of t/ssl/proxy.t.


The same here on Solaris 10 Sparc.


The request times out with a 504 status. So it looks like the "backend" in this 
request does not respond.
Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
closeset guess would be that the changes to
checkpipeline cause this.


Regards,

Rainer

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Yann Ylavic]

On Tue, Mar 15, 2016 at 12:42 AM, Graham Leggett  wrote:
> On 14 Mar 2016, at 10:48 AM, Ruediger Pluem  wrote:
>
>> This seems to cause frequent (no always) failures with test 8 of 
>> t/ssl/proxy.t.
>> The request times out with a 504 status. So it looks like the "backend" in 
>> this request does not respond.
>> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
>> closeset guess would be that the changes to
>> checkpipeline cause this.
>
> I commented out check_pipeline() completely, and it passed all the tests.
>
> Looking at check_pipeline, it seems to try and eat trailing CRLFs, which is 
> duplicating the functionality in read_request_line() which eats preceding 
> CRLFs already.
>
> check_pipeline() seems to have been overhauled recently, not sure what 
> problem it is trying to solve. Can we remove it?

No we can't, or at least we need to eat the trailing CRLFs.
Otherwise they will be considered as a pipelined request, thus if no
request is really following, we will block in read_request_line(), w/o
FLUSHing the previous response.
For mpm event, that also causes a spurious wake up (defeating
non-blocking since read_request_line() is/was? blocking).

But maybe your recent changes could avoid this, dunno, at least we
need to flush when there is no real pipelined request already there
(after the CRLFs).

Regards,
Yann.

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Graham Leggett]

On 14 Mar 2016, at 10:48 AM, Ruediger Pluem  wrote:

> This seems to cause frequent (no always) failures with test 8 of 
> t/ssl/proxy.t.
> The request times out with a 504 status. So it looks like the "backend" in 
> this request does not respond.
> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
> closeset guess would be that the changes to
> checkpipeline cause this.

I commented out check_pipeline() completely, and it passed all the tests.

Looking at check_pipeline, it seems to try and eat trailing CRLFs, which is 
duplicating the functionality in read_request_line() which eats preceding CRLFs 
already.

check_pipeline() seems to have been overhauled recently, not sure what problem 
it is trying to solve. Can we remove it?

Regards,
Graham
—

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Jim Jagielski]

They are also proxy-timeout related it seems:

# testing : QUERY_STRING passed OK
# expected: qr/QUERY_STRING = horse=trigger\n/s
# received: '
# 
# 504 Proxy Error
# 
# Proxy Error
# The gateway did not receive a timely response
# from the upstream server or application.
# '
not ok 27
# Failed test 27 in t/modules/rewrite.t at line 85

> On Mar 14, 2016, at 10:03 AM, Ruediger Pluem  wrote:
> 
> 
> 
> On 03/14/2016 02:55 PM, Jim Jagielski wrote:
>> 
>>> On Mar 14, 2016, at 4:48 AM, Ruediger Pluem  wrote:
>>> 
>>> 
>>> This seems to cause frequent (no always) failures with test 8 of 
>>> t/ssl/proxy.t.
>>> The request times out with a 504 status. So it looks like the "backend" in 
>>> this request does not respond.
>>> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
>>> closeset guess would be that the changes to
>>> checkpipeline cause this.
>>> 
>>> 
>> 
>> Getting similar behavior on OSX as well. Note sure if you are also
>> seeing occasional errors here as well:
>>   t/modules/rewrite.t . 28/29 # Failed test 28 in 
>> t/modules/rewrite.t at line 88
>>   # Failed test 29 in t/modules/rewrite.t at line 90
>> 
>> Also:
>>   t/modules/proxy.t ... 5/18 # Failed test 5 in 
>> t/modules/proxy.t at line 28
>>   # Failed test 6 in t/modules/proxy.t at line 29
>> 
>> 
>> 
>> 
> 
> I have not noticed these here.
> 
> Regards
> 
> Rüdiger
> 

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Ruediger Pluem]

On 03/14/2016 02:55 PM, Jim Jagielski wrote:
> 
>> On Mar 14, 2016, at 4:48 AM, Ruediger Pluem  wrote:
>>
>>
>> This seems to cause frequent (no always) failures with test 8 of 
>> t/ssl/proxy.t.
>> The request times out with a 504 status. So it looks like the "backend" in 
>> this request does not respond.
>> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
>> closeset guess would be that the changes to
>> checkpipeline cause this.
>>
>>
> 
> Getting similar behavior on OSX as well. Note sure if you are also
> seeing occasional errors here as well:
>t/modules/rewrite.t . 28/29 # Failed test 28 in 
> t/modules/rewrite.t at line 88
># Failed test 29 in t/modules/rewrite.t at line 90
> 
> Also:
>t/modules/proxy.t ... 5/18 # Failed test 5 in 
> t/modules/proxy.t at line 28
># Failed test 6 in t/modules/proxy.t at line 29
> 
> 
> 
> 

I have not noticed these here.

Regards

Rüdiger

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Jim Jagielski]

> On Mar 14, 2016, at 4:48 AM, Ruediger Pluem  wrote:
> 
> 
> This seems to cause frequent (no always) failures with test 8 of 
> t/ssl/proxy.t.
> The request times out with a 504 status. So it looks like the "backend" in 
> this request does not respond.
> Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
> closeset guess would be that the changes to
> checkpipeline cause this.
> 
> 

Getting similar behavior on OSX as well. Note sure if you are also
seeing occasional errors here as well:
   t/modules/rewrite.t . 28/29 # Failed test 28 in 
t/modules/rewrite.t at line 88
   # Failed test 29 in t/modules/rewrite.t at line 90

Also:
   t/modules/proxy.t ... 5/18 # Failed test 5 in 
t/modules/proxy.t at line 28
   # Failed test 6 in t/modules/proxy.t at line 29

Re: svn commit: r1734656 - in /httpd/httpd/trunk: ./ include/ modules/http/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/

[Ruediger Pluem]

On 03/12/2016 01:43 AM, minf...@apache.org wrote:
> Author: minfrin
> Date: Sat Mar 12 00:43:58 2016
> New Revision: 1734656
> 
> URL: http://svn.apache.org/viewvc?rev=1734656=rev
> Log:
> core: Extend support for setting aside data from the network input filter
> to any connection or request input filter.
> 
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/include/mpm_common.h
> httpd/httpd/trunk/include/util_filter.h
> httpd/httpd/trunk/modules/http/http_core.c
> httpd/httpd/trunk/modules/http/http_request.c
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/core_filters.c
> httpd/httpd/trunk/server/mpm/event/event.c
> httpd/httpd/trunk/server/mpm/motorz/motorz.c
> httpd/httpd/trunk/server/mpm/simple/simple_io.c
> httpd/httpd/trunk/server/mpm_common.c
> httpd/httpd/trunk/server/util_filter.c
> 


This seems to cause frequent (no always) failures with test 8 of t/ssl/proxy.t.
The request times out with a 504 status. So it looks like the "backend" in this 
request does not respond.
Used MPM is Event, OS CentOS 6 64 Bit. Without further investigating my 
closeset guess would be that the changes to
checkpipeline cause this.


Regards

Rüdiger