Re: svn commit: r1242351 - in /httpd/httpd/trunk: docs/manual/mod/mod_proxy.xml modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/mod_proxy_fcgi.c server/util_script.c

[Jeff Trawick <tr...@gmail.com>]

On Thu, Feb 9, 2012 at 11:39 AM, Jim Riggs <ap...@riggs.me> wrote:

> On Feb 9, 2012, at 10:22 AM, Ruediger Pluem wrote:
>
> > jim@apache.org wrote:
> >> Author: jim
> >> Date: Thu Feb  9 15:07:22 2012
> >> New Revision: 1242351
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1242351&view=rev
> >> Log:
> >> Handle cases, esp when using mod_proxy_fcgi, when we do not
> >> want SCRIPT_FILENAME to include the query string.
> >>
> >> Modified:
> >>    httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
> >>    httpd/httpd/trunk/modules/proxy/mod_proxy.c
> >>    httpd/httpd/trunk/modules/proxy/mod_proxy.h
> >>    httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
> >>    httpd/httpd/trunk/server/util_script.c
> >>
> >
> >> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
> >> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c?rev=1242351&r1=1242350&r2=1242351&view=diff
> >>
> ==============================================================================
> >> --- httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c (original)
> >> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c Thu Feb  9
> 15:07:22 2012
> >> @@ -188,7 +188,7 @@ static apr_status_t send_data(proxy_conn
> >>     while (to_write) {
> >>         apr_size_t n = 0;
> >>         rv = apr_socket_sendv(s, vec + offset, nvec - offset, &n);
> >> -        if (rv != APR_SUCCESS) {
> >> +        if ((rv != APR_SUCCESS) && !APR_STATUS_IS_EAGAIN(rv)) {
> >>             break;
> >>         }
> >>         if (n > 0) {
> >>
> >
> > How is this related to the log message and the query string issue?
>
> It appears this commit has to do with the message I sent the other day (
> http://mail-archives.apache.org/mod_mbox/httpd-dev/201202.mbox/%3C8EA4E5E4-F97E-410B-AAD4-257ECB4F972A@riggs.me%3E
> ).
>
> The question is, was that EAGAIN "fix" supposed to be included in this
> commit or not? Is that even the right "fix"? That was my brute-force
> attempt to address the issue I was having, but I don't know if it is
> correct. Do we need to check in the loop for a timeout or something in case
> we keep receiving EAGAIN? Or would the timeout get picked up and bail out
> elsewhere?
>
>
I'm sure that the check for APR_STATUS_IS_EAGAIN() helped the external
behavior (since the caller asked for non-blocking I/O but didn't handle
EAGAIN), but this change alone results in a busy-loop until the TCP layer
is kind enough to take more data.

E.g.:

644073 15:41:37.771942532 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=8
644076 15:41:37.771952171 1 httpd (15752) < writev res=8 data=........
644081 15:41:37.771991878 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=8200
644088 15:41:37.772019664 1 httpd (15752) < writev res=4344 data=.... ...
644089 15:41:37.772021067 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=3856
644090 15:41:37.772022788 1 httpd (15752) < writev res=-11(EAGAIN) data= <a
href=\"#rewriteengine\">RewriteEngine</a></li>.<li><img alt=\"\"
src=\"../images/
644091 15:41:37.772023931 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=3856
644092 15:41:37.772024933 1 httpd (15752) < writev res=-11(EAGAIN) data= <a
href=\"#rewriteengine\">RewriteEngine</a></li>.<li><img alt=\"\"
src=\"../images/
644093 15:41:37.772025721 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=3856
644094 15:41:37.772026683 1 httpd (15752) < writev res=-11(EAGAIN) data= <a
href=\"#rewriteengine\">RewriteEngine</a></li>.<li><img alt=\"\"
src=\"../images/
644095 15:41:37.772027472 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=3856
644096 15:41:37.772028434 1 httpd (15752) < writev res=-11(EAGAIN) data= <a
href=\"#rewriteengine\">RewriteEngine</a></li>.<li><img alt=\"\"
src=\"../images/
644097 15:41:37.772029239 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=3856
644098 15:41:37.772030198 1 httpd (15752) < writev res=-11(EAGAIN) data= <a
href=\"#rewriteengine\">RewriteEngine</a></li>.<li><img alt=\"\"
src=\"../images/
644099 15:41:37.772031007 1 httpd (15752) > writev
fd=11(<4t>192.168.1.210:41223->192.168.1.211:2003) size=3856

and on and on and on and on.

(Yeah, I like to use mod_rewrite.html.en as a big request or response body
for some reason.)

When send_data() was changed in r390571 to support non-blocking writes, the
single caller of send_data() which enabled that didn't handle EAGAIN, so
the non-blocking feature apparently never worked.  I'll yank that logic
now.  (That's the extra parameter to send_data(), the non-blocking
enablement/disablement, and checking for EAGAIN.)

-- 
Born in Roswell... married an alien...
http://emptyhammock.com/
http://edjective.org/