You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Ames <gr...@remulak.net> on 2001/07/25 00:07:03 UTC
Apache/2.0.22-dev is running on apache.org
...with code from cvs as of late this afternoon.
This build contains:
* a fix for the seg faults in mod_status, which Brian and Jeff reported
to me independently,
* Justin's small change to apr_sendfile for FreeBSD,
* an assert() trap, also in apr_sendfile, for the problem which is
responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
sent - wtf??), and
* all the other Good Stuff that's been committed lately.
This build also has a some other important fixes (gethostbyname
w/threaded on Solaris and htpasswd.c) which I can't test on daedalus.
Greg
Re: Apache/2.0.22-dev is running on apache.org
Posted by Greg Ames <gr...@remulak.net>.
Justin Erenkrantz wrote:
> But, that's an awful lot of extra code. Couldn't we somehow
> consolidate the logic? Maybe loop when:
> rv == -1 && (errno == EAGAIN || errno == EINTR)?
I agree. That's exactly what went into last night's build on daedalus,
and what I just committed.
I assume that Solaris's apr_sendfile could stand similar tweaking.
Greg
Re: Apache/2.0.22-dev is running on apache.org
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Jul 25, 2001 at 09:16:28PM -0400, Jeff Trawick wrote:
> no, not a problem.
>
> When EAGAIN sends something we promptly set rv to 0 so we avoid the
> wait_for_io_or_timeout()/re-try path.
>
> There is no need to adjust the parameters since we do
> wait_for_io_or_timeout()/retry *only* if we didn't send anything the
> first time. This is the same way the other send-type calls work in
> APR.
Yeah, I see that now. I'll blame the bad bandwidth at the O'Reilly
conference for causing me not to see that...
But, that's an awful lot of extra code. Couldn't we somehow
consolidate the logic? Maybe loop when:
rv == -1 && (errno == EAGAIN || errno == EINTR)?
I'll look at it in the morning and see if I can come up with anything
obvious. This duplication just rubs me the wrong way. =) -- justin
Re: Apache/2.0.22-dev is running on apache.org
Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz <je...@ebuilt.com> writes:
> On Wed, Jul 25, 2001 at 04:12:13PM -0400, Jeff Trawick wrote:
> > Jeff Trawick <tr...@attglobal.net> writes:
> >
> > > Greg Ames <gr...@remulak.net> writes:
> > >
> > > > But what if we have rv == -1, EAGAIN, and nbytes == 0 ? I don't think
> > > > we handle that properly yet in FreeBSD, nor in Solaris. Looks to me
> > > > like the new FreeBSD code will exit apr_sendfile with APR_EAGAIN, and
> > > > sendfile_it_all will bail, and the request will die.
> > >
> > > yeah, it definitely looks busted... I'll work on that...
> >
> > Here is a patch for FreeBSD which I'll commit soon if nobody
> > complains. It fixes a few problems with when we call
> > wait_for_io_or_timeout(), and it also tries to send again when
> > wait_for_io_or_timeout() succeeds.
>
> [I'm in San Diego right now for the O'Reilly conference, so this
> will be short.]
>
> This doesn't work because you now need to adjust the sendfile params.
> On FreeBSD/Solaris, EAGAIN means that it sent something - merely
> repeating the call to sendfile with the same parameters is bogus
> (same data sent twice).
no, not a problem.
When EAGAIN sends something we promptly set rv to 0 so we avoid the
wait_for_io_or_timeout()/re-try path.
There is no need to adjust the parameters since we do
wait_for_io_or_timeout()/retry *only* if we didn't send anything the
first time. This is the same way the other send-type calls work in
APR.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: Apache/2.0.22-dev is running on apache.org
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Jul 25, 2001 at 04:12:13PM -0400, Jeff Trawick wrote:
> Jeff Trawick <tr...@attglobal.net> writes:
>
> > Greg Ames <gr...@remulak.net> writes:
> >
> > > But what if we have rv == -1, EAGAIN, and nbytes == 0 ? I don't think
> > > we handle that properly yet in FreeBSD, nor in Solaris. Looks to me
> > > like the new FreeBSD code will exit apr_sendfile with APR_EAGAIN, and
> > > sendfile_it_all will bail, and the request will die.
> >
> > yeah, it definitely looks busted... I'll work on that...
>
> Here is a patch for FreeBSD which I'll commit soon if nobody
> complains. It fixes a few problems with when we call
> wait_for_io_or_timeout(), and it also tries to send again when
> wait_for_io_or_timeout() succeeds.
[I'm in San Diego right now for the O'Reilly conference, so this
will be short.]
This doesn't work because you now need to adjust the sendfile params.
On FreeBSD/Solaris, EAGAIN means that it sent something - merely
repeating the call to sendfile with the same parameters is bogus
(same data sent twice).
You need to adjust the parameters. It's probably cheaper
to bail out and tell the caller how much was sent. That was how
I interpeted the use cases of apr_sendfile. There is no contract that
it'll send everything in one call. -- justin
Re: Apache/2.0.22-dev is running on apache.org
Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:
> Greg Ames <gr...@remulak.net> writes:
>
> > But what if we have rv == -1, EAGAIN, and nbytes == 0 ? I don't think
> > we handle that properly yet in FreeBSD, nor in Solaris. Looks to me
> > like the new FreeBSD code will exit apr_sendfile with APR_EAGAIN, and
> > sendfile_it_all will bail, and the request will die.
>
> yeah, it definitely looks busted... I'll work on that...
Here is a patch for FreeBSD which I'll commit soon if nobody
complains. It fixes a few problems with when we call
wait_for_io_or_timeout(), and it also tries to send again when
wait_for_io_or_timeout() succeeds.
Index: srclib/apr/network_io/unix/sendrecv.c
===================================================================
RCS file: /home/cvspublic/apr/network_io/unix/sendrecv.c,v
retrieving revision 1.71
diff -u -r1.71 sendrecv.c
--- srclib/apr/network_io/unix/sendrecv.c 2001/07/25 05:27:32 1.71
+++ srclib/apr/network_io/unix/sendrecv.c 2001/07/25 20:04:59
@@ -447,24 +447,51 @@
}
} while (rv == -1 && errno == EINTR);
- /* On FreeBSD, it is possible that sendfile will return EAGAIN, but
- * still send some data. This means that we cannot call sendfile
- * and then check for EAGAIN, and then wait and call sendfile again.
- * If we do that, then we are likely to send the first chunk of data
- * twice, once in the first call and once in the second.
- *
- * When we are dealing with a non-blocking or timeout socket, the
- * caller must already be aware that we may not be able to write
- * everything in one call. Therefore, we should return back to
- * the caller with how much we actually sent (as specified from EAGAIN).
- *
- * If we are using a timed write, we will now block until we are clear.
- */
- if (errno == EAGAIN && nbytes && sock->timeout >= 0) {
+ if (rv == -1 &&
+ errno == EAGAIN &&
+ sock->timeout > 0) {
apr_status_t arv = apr_wait_for_io_or_timeout(sock, 0);
if (arv != APR_SUCCESS) {
*len = 0;
return arv;
+ }
+ else {
+ do {
+ if (bytes_to_send) {
+ /* We won't dare call sendfile() if we don't have
+ * header or file bytes to send because bytes_to_send == 0
+ * means send the whole file.
+ */
+ rv = sendfile(file->filedes, /* file to be sent */
+ sock->socketdes, /* socket */
+ *offset, /* where in the file to start */
+ bytes_to_send, /* number of bytes to send */
+ &headerstruct, /* Headers/footers */
+ &nbytes, /* number of bytes written */
+ flags); /* undefined, set to 0 */
+ /* FreeBSD's sendfile can return -1/EAGAIN even if it
+ * sent bytes. Sanitize the result so we get normal EAGAIN
+ * semantics w.r.t. bytes sent.
+ */
+ if (rv == -1 && errno == EAGAIN && nbytes) {
+ rv = 0;
+ }
+ }
+ else {
+ /* just trailer bytes... use writev()
+ */
+ rv = writev(sock->socketdes,
+ hdtr->trailers,
+ hdtr->numtrailers);
+ if (rv > 0) {
+ nbytes = rv;
+ rv = 0;
+ }
+ else {
+ nbytes = 0;
+ }
+ }
+ } while (rv == -1 && errno == EINTR);
}
}
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: Apache/2.0.22-dev is running on apache.org
Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Ames <gr...@remulak.net> writes:
> But what if we have rv == -1, EAGAIN, and nbytes == 0 ? I don't think
> we handle that properly yet in FreeBSD, nor in Solaris. Looks to me
> like the new FreeBSD code will exit apr_sendfile with APR_EAGAIN, and
> sendfile_it_all will bail, and the request will die.
yeah, it definitely looks busted... I'll work on that...
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: Apache/2.0.22-dev is running on apache.org
Posted by Greg Ames <gr...@remulak.net>.
Jeff Trawick wrote:
>
> Justin Erenkrantz <je...@ebuilt.com> writes:
>
> > On Tue, Jul 24, 2001 at 06:07:03PM -0400, Greg Ames wrote:
> > > * an assert() trap, also in apr_sendfile, for the problem which is
> > > responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> > > sent - wtf??), and
> >
> > Could this be from sendfile returning EAGAIN?
>
> Presumably, this piece of code I added a while back would take care of
> that:
>
> /* FreeBSD's sendfile can return -1/EAGAIN even if it
> * sent bytes. Sanitize the result so we get normal EAGAIN
> * semantics w.r.t. bytes sent.
> */
> if (rv == -1 && errno == EAGAIN && nbytes) {
> rv = 0;
> }
>
After lots of staring at apr_sendfile for FreeBSD last night, my head
hurt. So I took advantage of the situation and had a Pain Killer.*
This code snippet doesn't address the core dumps, and doesn't address
Justin's question either, because nbytes must be non-zero. When we
dump, we have nbytes == 0 and rv == 0. My new trap should get us closer
to that problem.
But what if we have rv == -1, EAGAIN, and nbytes == 0 ? I don't think
we handle that properly yet in FreeBSD, nor in Solaris. Looks to me
like the new FreeBSD code will exit apr_sendfile with APR_EAGAIN, and
sendfile_it_all will bail, and the request will die.
However, the new build seems to be running like a champ on FreeBSD.
Maybe we never/rarely see the EAGAIN/nbytes == 0 combination on
daedalus; the FreeBSD kernel might always send out some bytes when it
returns EAGAIN. If we really need to find out what's going on here, I
could put in a couple of counters in shared memory - one for EAGAINs,
one for the EAGAIN/0 bytes combination.
I like the look of the HP apr_sendfile better. If it sees EAGAIN, it
waits for writeability, then calls sendfile a second time.
> We do lots of sendfiles on apache.org but only very occasionally
> (never more than once a day, usually much less often) it fails.
True.
Greg
*Pain Killer
3 shots pineapple juice
2 shots coconut cream
1 shot orange juice
x shots dark rum - I prefer Gosling's Black Seal
(x is proportional to the severity of the pain)
serve over ice. Sprinkle nutmeg on top.
created by:
The Soggy Dollar Bar
(no dock, so you swim ashore from your boat, hence the name)
White Bay
Jost Van Dyke
British Virgin Islands
Re: Apache/2.0.22-dev is running on apache.org
Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz <je...@ebuilt.com> writes:
> On Tue, Jul 24, 2001 at 06:07:03PM -0400, Greg Ames wrote:
> > * an assert() trap, also in apr_sendfile, for the problem which is
> > responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> > sent - wtf??), and
>
> Could this be from sendfile returning EAGAIN?
Presumably, this piece of code I added a while back would take care of
that:
/* FreeBSD's sendfile can return -1/EAGAIN even if it
* sent bytes. Sanitize the result so we get normal EAGAIN
* semantics w.r.t. bytes sent.
*/
if (rv == -1 && errno == EAGAIN && nbytes) {
rv = 0;
}
We do lots of sendfiles on apache.org but only very occasionally
(never more than once a day, usually much less often) it fails.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: Apache/2.0.22-dev is running on apache.org
Posted by Greg Ames <gr...@remulak.net>.
Dirk-Willem van Gulik wrote:
>
> On Tue, 24 Jul 2001, Justin Erenkrantz wrote:
>
> > On Tue, Jul 24, 2001 at 06:07:03PM -0400, Greg Ames wrote:
> > > * an assert() trap, also in apr_sendfile, for the problem which is
> > > responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> > > sent - wtf??), and
> >
> > Could this be from sendfile returning EAGAIN?
> >
> > When testing it on Solaris, sendfilev would return EAGAIN and 0 bytes
> > sent if the socket isn't ready yet. (FreeBSD has the closest
> > semantics to Solaris as any others out there.) -- justin
>
> Hmm - are you sure you do not get an EPIPE in that case ? Peraps it should
> be
> /* FreeBSD's sendfile can return -1/EAGAIN even if it
> * sent bytes. Sanitize the result so we get normal EAGAIN
> * semantics w.r.t. bytes sent.
> */
> if ((rv == -1) &&
> ((errno == EAGAIN) || (errno == EPIPE )) &&
> (nbytes))
> {
> rv = 0;
> }
I don't think so. EPIPE usually means the connection is gone,
permanently. We just return the error to the caller, and the request
dies, as it should.
Greg
Re: Apache/2.0.22-dev is running on apache.org
Posted by Dirk-Willem van Gulik <di...@covalent.net>.
On Tue, 24 Jul 2001, Justin Erenkrantz wrote:
> On Tue, Jul 24, 2001 at 06:07:03PM -0400, Greg Ames wrote:
> > * an assert() trap, also in apr_sendfile, for the problem which is
> > responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> > sent - wtf??), and
>
> Could this be from sendfile returning EAGAIN?
>
> When testing it on Solaris, sendfilev would return EAGAIN and 0 bytes
> sent if the socket isn't ready yet. (FreeBSD has the closest
> semantics to Solaris as any others out there.) -- justin
Hmm - are you sure you do not get an EPIPE in that case ? Peraps it should
be
/* FreeBSD's sendfile can return -1/EAGAIN even if it
* sent bytes. Sanitize the result so we get normal EAGAIN
* semantics w.r.t. bytes sent.
*/
if ((rv == -1) &&
((errno == EAGAIN) || (errno == EPIPE )) &&
(nbytes))
{
rv = 0;
}
Dw
Re: Apache/2.0.22-dev is running on apache.org
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 24, 2001 at 06:07:03PM -0400, Greg Ames wrote:
> * an assert() trap, also in apr_sendfile, for the problem which is
> responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> sent - wtf??), and
Could this be from sendfile returning EAGAIN?
When testing it on Solaris, sendfilev would return EAGAIN and 0 bytes
sent if the socket isn't ready yet. (FreeBSD has the closest
semantics to Solaris as any others out there.) -- justin
Re: Apache/2.0.22-dev is running on apache.org
Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Ames <gr...@remulak.net> writes:
> ...with code from cvs as of late this afternoon.
>
> This build contains:
>
> * a fix for the seg faults in mod_status, which Brian and Jeff reported
> to me independently,
> * Justin's small change to apr_sendfile for FreeBSD,
> * an assert() trap, also in apr_sendfile, for the problem which is
> responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> sent - wtf??), and
> * all the other Good Stuff that's been committed lately.
>
> This build also has a some other important fixes (gethostbyname
> w/threaded on Solaris and htpasswd.c) which I can't test on daedalus.
There is also the important APR_INHERIT fix. It looks to me like you
hit that problem when you brought down apache-2.0.21 on apache.org
today in preparation for bringing up 2.0.22.
See /usr/local/apache2.0.21/corefiles/httpd.core.3.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: Apache/2.0.22-dev is running on apache.org
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 24, 2001 at 06:17:26PM -0400, Cliff Woolley wrote:
> 2.0.22, anyone? I smell a beta on the horizon...
+1. The threaded MPM is a bit saner now. We've got most of the
threading issues on Solaris under control now.
I'm still uncertain if we need to be calling pthread_setconcurrency
somewhere in the threaded MPM. I'll try to take a look at it tonight.
And, if you've got Solaris 8 with the sendfilev() patches, you've
probably got a major improvement since 2.0.21. -- justin
Re: Apache/2.0.22-dev is running on apache.org
Posted by Jeff Trawick <tr...@attglobal.net>.
Cliff Woolley <cl...@yahoo.com> writes:
> On Tue, 24 Jul 2001, Greg Ames wrote:
>
> > This build contains:
> >
> > * a fix for the seg faults in mod_status, which Brian and Jeff reported
> > to me independently,
> > * Justin's small change to apr_sendfile for FreeBSD,
> > * an assert() trap, also in apr_sendfile, for the problem which is
> > responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> > sent - wtf??), and
> > * all the other Good Stuff that's been committed lately.
> >
> > This build also has a some other important fixes (gethostbyname
> > w/threaded on Solaris and htpasswd.c) which I can't test on daedalus.
>
> 2.0.22, anyone? I smell a beta on the horizon...
yeah, maybe we should tag now in anticipation, before anybody breaks
anything :)
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: Apache/2.0.22-dev is running on apache.org
Posted by Jeff Trawick <tr...@attglobal.net>.
Dirk-Willem van Gulik <di...@covalent.net> writes:
> > We might want to wait for some of the conference-goers to get a
> > chance to get back to their mail clients (in case they have some
> > outstanding patches they'd like to get in *wink-wink*).
>
> Working fine here too (freebsd and drawin). But still some small issues
> with occasional coredump on shutdown in the pool/memory mngt code. I've
> got plenty of cores; but still no idea where to look.
I was getting this until OtherBill fixed the inherit stuff. The core
dumps I saw at shutdown were due to apr_pools trying to call
(*c)->plain_cleanup, where plain_cleanup was NULL.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: Apache/2.0.22-dev is running on apache.org
Posted by Dirk-Willem van Gulik <di...@covalent.net>.
> We might want to wait for some of the conference-goers to get a
> chance to get back to their mail clients (in case they have some
> outstanding patches they'd like to get in *wink-wink*).
Working fine here too (freebsd and drawin). But still some small issues
with occasional coredump on shutdown in the pool/memory mngt code. I've
got plenty of cores; but still no idea where to look.
Dw
Re: Apache/2.0.22-dev is running on apache.org
Posted by Aaron Bannert <aa...@ebuilt.com>.
We might want to wait for some of the conference-goers to get a
chance to get back to their mail clients (in case they have some
outstanding patches they'd like to get in *wink-wink*).
-aaron
On Tue, Jul 24, 2001 at 07:44:59PM -0400, Bill Stoddard wrote:
> +1 on tag and roll
Re: Apache/2.0.22-dev is running on apache.org
Posted by Bill Stoddard <bi...@wstoddard.com>.
+1 on tag and roll
Bill
> On Tue, 24 Jul 2001, Greg Ames wrote:
>
> > This build contains:
> >
> > * a fix for the seg faults in mod_status, which Brian and Jeff reported
> > to me independently,
> > * Justin's small change to apr_sendfile for FreeBSD,
> > * an assert() trap, also in apr_sendfile, for the problem which is
> > responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> > sent - wtf??), and
> > * all the other Good Stuff that's been committed lately.
> >
> > This build also has a some other important fixes (gethostbyname
> > w/threaded on Solaris and htpasswd.c) which I can't test on daedalus.
>
> 2.0.22, anyone? I smell a beta on the horizon...
>
> --Cliff
>
>
> --------------------------------------------------------------
> Cliff Woolley
> cliffwoolley@yahoo.com
> Charlottesville, VA
>
>
Re: Apache/2.0.22-dev is running on apache.org
Posted by Cliff Woolley <cl...@yahoo.com>.
On Tue, 24 Jul 2001, Greg Ames wrote:
> This build contains:
>
> * a fix for the seg faults in mod_status, which Brian and Jeff reported
> to me independently,
> * Justin's small change to apr_sendfile for FreeBSD,
> * an assert() trap, also in apr_sendfile, for the problem which is
> responsible for the most core dumps on daedalus (APR_SUCCESS + 0 bytes
> sent - wtf??), and
> * all the other Good Stuff that's been committed lately.
>
> This build also has a some other important fixes (gethostbyname
> w/threaded on Solaris and htpasswd.c) which I can't test on daedalus.
2.0.22, anyone? I smell a beta on the horizon...
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA