Re: Event MPM: Spinning on cleanups?
On Dec 4, 2005, at 11:14 PM, Paul Querna wrote: I finally got around to upgrading to trunk w/ the Event MPM on one of my machines. Within a couple hours of starting it, I had a process spinning, and consuming 100% CPU. Backtrace from the spinning thread: (gdb) thread 6 [Switching to thread 6 (Thread 0x2045 (LWP 100189))]#0 apr_allocator_free (allocator=0x2054ab80, node=0x205a2000) at memory/unix/ apr_pools.c:334 334 if (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED (gdb) where #0 apr_allocator_free (allocator=0x2054ab80, node=0x205a2000) at memory/unix/apr_pools.c:334 Paul, if you're able to reproduce this condition, can you post a dump of the free lists in the allocator? If something allocated from the apr_allocator_t is being freed twice, it may be detectable as a loop in one of the allocator->free[] lists. Thanks, Brian
apr_allocator Re: Event MPM: Spinning on cleanups?
On Dec 30, 2005, at 6:48 PM, Roy T. Fielding wrote:
On Dec 30, 2005, at 5:51 PM, Brian Pane wrote:
I haven't been able to find the bug yet. As a next step, I'll try
using
valgrind on a build with pool debugging enabled.
On entry to allocator_free, if
(node == node->next && node->index > current_free_index)
is true, then the do { } while ((node = next) != NULL);
will go into an infinite loop. This is because
if (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED
&& index > current_free_index) {
node->next = freelist;
freelist = node;
}
does not update current_free_index. I don't know if that is the
problem, but it may be the symptom.
After reading and instrumenting the apr_allocator code, I just found
that odd things happen when
max_free_index == APR_ALLOCATOR_MAX_FREE_UNLIMITED.
The doesn't appear to be the cause of the problem in the Event
MPM, but it's worth noting.
APR_ALLOCATOR_MAX_FREE_UNLIMITED is zero, and it's used
as the initial value for allocator->current_free_index. The problem is
that when apr_allocator_free() stores a block in one of its free
lists, it
subtracts that block's index (a value from 1 to 20) from
allocator->current_free_index.
Note that allocator->current_free_index is unsigned. Thus, when
it starts out at 0, subsequent calls to apr_allocator_free() turn it
into
a number slightly smaller than 2^32. On the next call to
apr_allocator_alloc() that recycles a block from one of the free
lists, this bit of defensive code ends up returning the value of
allocator->current_free_index to zero:
allocator->current_free_index += node->index;
if (allocator->current_free_index > allocator-
>max_free_index)
allocator->current_free_index = allocator-
>max_free_index;
We probably should have a check in apr_allocator_free() to
make sure that we're not causing an unsigned int overflow in
allocator->current_free_index, e.g.,
+if (index > current_tree_index) {
+ current_tree_index = 0;
+}
+else {
current_free_index -= index;
+}
In cases where max_free_index is not set to
APR_ALLOCATOR_MAX_FREE_UNLIMITED, all the
current_free_index logic appears to work in a sane
manner: the values of current_free_index and
max_free_index are basically base-2 logarithms
of the amount of space currently in the
allocator->free[] lists and of the max amount
of space that may be stored in these lists.
These variables really could use clearer names
and some descriptive comments, however. They're
not really indices: even without the unsigned
int overflow problem, they can have values much
larger than MAX_INDEX. Maybe it's just me, but
the naming made it very tough to figure out the
underlying powers-of-two trick.
I'll commit a patch when I have time, hopefully
in the next couple of days.
Brian
Re: Event MPM: Spinning on cleanups?
On Dec 30, 2005, at 5:51 PM, Brian Pane wrote:
I haven't been able to find the bug yet. As a next step, I'll try
using
valgrind on a build with pool debugging enabled.
On entry to allocator_free, if
(node == node->next && node->index > current_free_index)
is true, then the do { } while ((node = next) != NULL);
will go into an infinite loop. This is because
if (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED
&& index > current_free_index) {
node->next = freelist;
freelist = node;
}
does not update current_free_index. I don't know if that is the
problem, but it may be the symptom.
Roy
Re: Event MPM: Spinning on cleanups?
I haven't been able to find the bug yet. As a next step, I'll try using valgrind on a build with pool debugging enabled. Brian On Dec 4, 2005, at 11:14 PM, Paul Querna wrote: I finally got around to upgrading to trunk w/ the Event MPM on one of my machines. Within a couple hours of starting it, I had a process spinning, and consuming 100% CPU. Backtrace from the spinning thread: (gdb) thread 6 [Switching to thread 6 (Thread 0x2045 (LWP 100189))]#0 apr_allocator_free (allocator=0x2054ab80, node=0x205a2000) at memory/unix/ apr_pools.c:334 334 if (max_free_index != APR_ALLOCATOR_MAX_FREE_UNLIMITED (gdb) where #0 apr_allocator_free (allocator=0x2054ab80, node=0x205a2000) at memory/unix/apr_pools.c:334 #1 0x280eb952 in apr_bucket_free (mem=0x0) at buckets/apr_buckets_alloc.c:182 #2 0x280e9915 in heap_bucket_destroy (data=0x205a4090) at buckets/apr_buckets_heap.c:36 #3 0x280e9a54 in apr_brigade_cleanup (data=0x2059e6b0) at buckets/apr_brigade.c:44 #4 0x280e9a8b in brigade_cleanup (data=0x2059e6b0) at buckets/apr_brigade.c:34 #5 0x282021bd in run_cleanups (cref=0x2059e028) at memory/unix/apr_pools.c:2044 #6 0x28202f39 in apr_pool_clear (pool=0x2059e018) at memory/unix/apr_pools.c:689 #7 0x08081063 in worker_thread (thd=0x81d1660, dummy=0x0) at event.c:682 #8 0x2820b3e4 in dummy_worker (opaque=0x0) at threadproc/unix/ thread.c:138 #9 0x2823720b in pthread_create () from /usr/lib/libpthread.so.2 #10 0x282fa1ef in _ctx_start () from /lib/libc.so.6 OS: FreeBSD 6.0-STABLE APR: Trunk APR-Util: Trunk HTTPD: Trunk Best guess is that we corrupted a bucket brigade by double freeing it, or something of that kind. This is definitely a new behavior since the async-write code was merged into trunk. It is odd that we could of double-free'ed something on the connection pool. Maybe it isn't a double-free issue at all... I'm too tired to debug much of it tonight. Maybe later this week I will dig deeper. -Paul
[PATCH] mod_proxy_fcgi - fix calcuation of request id and content length
There's a small bug in the fastcgi header parsing code, the chars need
to be treated as unsigned in order for all the shifting to work
properly...
Log follows, patch attached.
-garrett
Fix the extraction of shorts from the header parsing code.
* modules/proxy/mod_proxy_fcgi.c
(dispatch): Cast the parts of the short values to unsigned before shifting
and oring them together.
Index: modules/proxy/mod_proxy_fcgi.c
===
--- modules/proxy/mod_proxy_fcgi.c (revision 360174)
+++ modules/proxy/mod_proxy_fcgi.c (working copy)
@@ -482,8 +482,8 @@
type = readbuf[1];
-rid |= readbuf[2] << 8;
-rid |= readbuf[3] << 0;
+rid |= ((unsigned char) readbuf[2]) << 8;
+rid |= ((unsigned char) readbuf[3]) << 0;
if (rid != request_id) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
@@ -493,8 +493,8 @@
break;
}
-clen |= readbuf[4] << 8;
-clen |= readbuf[5] << 0;
+clen |= ((unsigned char) readbuf[4]) << 8;
+clen |= ((unsigned char) readbuf[5]) << 0;
plen = readbuf[6];
Re: [PATCH] mod_proxy_fcgi - handle content lengths larger than AP_IOBUFSIZE
On 12/30/05, Garrett Rooney <[EMAIL PROTECTED]> wrote:
> On 12/29/05, Garrett Rooney <[EMAIL PROTECTED]> wrote:
> > Here's a very lightly tested patch to allow mod_proxy_fcgi to deal
> > with fastcgi records with content length greater than AP_IOBUFSIZE.
> > If someone could double check the math to make sure it's correct in
> > all cases I'd appreciate it, I tested it by reducing the buffers to
> > very small sizes, and it seems to work fine.
>
> Ok, since Paul apparently saw some trouble with the django tutorial
> app in the first version of this patch, here's an updated version. It
> specifically null terminates the read buffer before checking for the
> end of headers, and switches to using readbuflen explicitly in the
> calculations for figuring out how much we have left to read.
>
> I tested this with a large variety of different buffer sizes, and the
> only known problem is that with certain cases we fail to find the end
> of the headers (when the \r\n\r\n falls on the edge between two
> reads), but that problem was already known. So if there's a problem
> other than that I'd love to get a small test case for it.
There still appears to be something funky in the logic for reading the
content and the padding in a single recv call... For now, here's a
third version that just puts off reading the padding till after the
content is done. We can revisit this and reduce the number of reads
later on when it becomes an issue, especially since not all fcgi
libraries even use padding bytes.
-garrett
Handle reading fastcgi records with content length larger than AP_IOBUFSIZE.
* modules/proxy/mod_proxy_fcgi.c
(proxy_fcgi_baton_t): New struct, holds per-connection data.
(dispatch): Set buckets aside into the scratch pool in the baton,
clearing it when we pass the baton on. Deal with the case where
the content length is larger than AP_IOBUFSIZE. Consistently use
sizeof when referring to the length of buffers. Explicitly null
terminate the read buffer after reading. Read the padding bytes
in a second pass to simplify logic.
(proxy_fcgi_handler): Create our baton and stash it in the connection's
data member.
Index: modules/proxy/mod_proxy_fcgi.c
===
--- modules/proxy/mod_proxy_fcgi.c (revision 359901)
+++ modules/proxy/mod_proxy_fcgi.c (working copy)
@@ -351,11 +351,16 @@
return 0;
}
+typedef struct {
+apr_pool_t *scratch_pool;
+} proxy_fcgi_baton_t;
+
static apr_status_t dispatch(proxy_conn_rec *conn, request_rec *r,
int request_id)
{
apr_bucket_brigade *ib, *ob;
int seen_end_of_headers = 0, done = 0;
+proxy_fcgi_baton_t *pfb = conn->data;
apr_status_t rv = APR_SUCCESS;
conn_rec *c = r->connection;
struct iovec vec[2];
@@ -388,7 +393,7 @@
rv = ap_get_brigade(r->input_filters, ib,
AP_MODE_READBYTES, APR_BLOCK_READ,
-AP_IOBUFSIZE);
+sizeof(writebuf));
if (rv != APR_SUCCESS) {
break;
}
@@ -496,33 +501,29 @@
/* Clear out the header so our buffer is zeroed out again */
memset(readbuf, 0, 8);
-/* XXX We need support for content length > buffer size, but for
- * now just punt. */
-if ((clen + plen) > sizeof(readbuf) - 1) {
-ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
- "proxy: FCGI: back end server send more data "
- "than fits in buffer");
-rv = APR_EINVAL;
-break;
+recv_again:
+if (clen > sizeof(readbuf) - 1) {
+readbuflen = sizeof(readbuf) - 1;
+} else {
+readbuflen = clen;
}
/* Now get the actual data. Yes it sucks to do this in a second
* recv call, this will eventually change when we move to real
* nonblocking recv calls. */
-if ((clen + plen) != 0) {
-readbuflen = clen + plen;
-
+if (readbuflen != 0) {
rv = apr_socket_recv(conn->sock, readbuf, &readbuflen);
if (rv != APR_SUCCESS) {
break;
}
+readbuf[readbuflen] = 0;
}
switch (type) {
case FCGI_STDOUT:
if (clen != 0) {
b = apr_bucket_transient_create(readbuf,
-clen,
+readbuflen,
c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, b);
@@ -548,11 +549,29 @@
}
apr_brigade_cleanup(ob);
+
+apr_pool_clear(pfb->scratch_
RAISE_SIGSTOP macro
Howdy, While reviewing the make_child code in prefork.c, I came across the RAISE_SIGSTOP macro. It looks like this macro allows you to utilize gdb to attach at specific locations based on the value of raise_sigstop_flags. I checked through the mailing list archives and saw that this code was committed by Dean Gaudet: http://216.239.51.104/search?q=cache:N9zuszgPn0IJ:www.mailarchives.org/list/apache_dev/msg/1997/13196+apache+RAISE_SIGSTOP&hl=en For whatever reason the '-Z' option has been removed from main.c, but the code that relies on this option still exists. Does anyone happen to know if this code is still used for debugging? Thanks for any insight, - Ryan -- UNIX Administrator http://daemons.net/~matty
Re: [PATCH] INSTALL
On Dec 21, 2005, at 12:34 PM, Justin Erenkrantz wrote: --On December 21, 2005 11:11:11 AM +0100 Sander Temme <[EMAIL PROTECTED]> wrote: Yes, but is that reworked remark about FreeBSD threads cool with you? Close, but not quite. Threads are enabled in APR by default on 5.4- RELEASE and higher (for APR 1.x branches which means httpd 2.2+). I don't think we use worker by default in any case, do we? So, that comment about using prefork instead is wrong: we do prefork by default everywhere. -- justin OK, I cleaned it up some more and committed as r359993. I compiled with and without threads, with prefork and worker on FreeBSD 6.0 and 4.5, and I think what's currently in INSTALL is True. Unless anyone objects, I'll backport the relevant bits of r359993 to the 2.2.x branch. I don't think this needs RTC because no code is touched. S. -- [EMAIL PROTECTED] http://www.temme.net/sander/ PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF smime.p7s Description: S/MIME cryptographic signature
Re: execd: fcgi, per-child, cgid and maybe suexec
Just this last one, and then that's it until mid-next-week :) On Dec 29, 2005, at 5:47 PM, Colm MacCarthaigh wrote: [1] and [3] on their own are simply enough, [2] is the crazy part. Does any of this make any sense? This all makes a lot of sense to me, in fact #2 kind of aligns with something I was thinking about regarding how to fit per-child into the proxy balancer stuff. In fact, I think that having proxy balancer be the front end to mod_execd allows for a lot of scaling and control, but I haven't gone any further than just thinking about it conceptually (something like defining a worker execd://foo.com/process). Yeah, we'll need something like apr_pass_fd/apr_grab_fd. This reminds me: I know that the GSoC effort for perchild didn't work out, but was *anything* done? I heard something about a design framework being produced?
Re: Apache 2.2.0 Listen Directive
On 12/28/05, Jeff Trawick <[EMAIL PROTECTED]> wrote:
> On 12/28/05, Fenlason, Josh <[EMAIL PROTECTED]> wrote:
> >
> > I'm running into an issue where Apache 2.2.0 on AIX won't start if there is
> > more than one Listen directive.
> Can you send me truss of startup using the failure configuration?
>
> truss -o /tmp/startup -f bin/apachectl start
(trace received offline)
I don't see any socket errors. I jumped more than half-way to a
conclusion from your initial report ("won't start") and assumed that
some sort of bind error occurred. It seems somewhat likely that a
crash is occurring, though I don't see SIGSEGV being reported in the
trace. Anything written to the console by "apachectl start"? What is
exit status of "apachectl start" in the failure?
# bin/apachectl start
# echo $?
Anything written to error log?
AIX 5.1 doesn't have IPV6_V6ONLY socket option (added in 5.2), which
does affect processing of sockets. Can you try this hack?
Index: server/listen.c
===
--- server/listen.c (revision 360100)
+++ server/listen.c (working copy)
@@ -408,11 +408,8 @@
if (previous != NULL
&& IS_INADDR_ANY(lr->bind_addr)
&& lr->bind_addr->port == previous->bind_addr->port
-&& IS_IN6ADDR_ANY(previous->bind_addr)
-&& apr_socket_opt_get(previous->sd, APR_IPV6_V6ONLY,
- &v6only_setting) == APR_SUCCESS
-&& v6only_setting == 0) {
-
+&& IS_IN6ADDR_ANY(previous->bind_addr)) {
+/* hacked to ignore IPV6_V6ONLY setting */
/* Remove the current listener from the list */
previous->next = lr->next;
continue;
Re: execd: fcgi, per-child, cgid and maybe suexec
On 12/29/05, Colm MacCarthaigh <[EMAIL PROTECTED]> wrote: > > [1] and [3] on their own are simply enough, [2] is the crazy part. > Does any of this make any sense? I don't know enough about [2] to say if it's possible or not, but it makes sense at first glance. I'm highly in favor of [3], since it means that Paul and I don't need to implement that part of it ourselves as part of the fastcgi work, so go for it! ;-) -garrett
Re: [PATCH] mod_proxy_fcgi - handle content lengths larger than AP_IOBUFSIZE
On 12/29/05, Garrett Rooney <[EMAIL PROTECTED]> wrote:
> Here's a very lightly tested patch to allow mod_proxy_fcgi to deal
> with fastcgi records with content length greater than AP_IOBUFSIZE.
> If someone could double check the math to make sure it's correct in
> all cases I'd appreciate it, I tested it by reducing the buffers to
> very small sizes, and it seems to work fine.
Ok, since Paul apparently saw some trouble with the django tutorial
app in the first version of this patch, here's an updated version. It
specifically null terminates the read buffer before checking for the
end of headers, and switches to using readbuflen explicitly in the
calculations for figuring out how much we have left to read.
I tested this with a large variety of different buffer sizes, and the
only known problem is that with certain cases we fail to find the end
of the headers (when the \r\n\r\n falls on the edge between two
reads), but that problem was already known. So if there's a problem
other than that I'd love to get a small test case for it.
-garrett
Handle reading fastcgi records with content length larger than AP_IOBUFSIZE.
* modules/proxy/mod_proxy_fcgi.c
(proxy_fcgi_baton_t): New struct, holds per-connection data.
(dispatch): Set buckets aside into the scratch pool in the baton,
clearing it when we pass the brigade on. Deal with the case where
the content length is larger than AP_IOBUFSIZE. Consistently use
sizeof when referring to the length of buffers. Explicitly null
terminate the read buffer after reading.
(proxy_fcgi_handler): Create our baton and stash it in the connection's
data member.
Index: modules/proxy/mod_proxy_fcgi.c
===
--- modules/proxy/mod_proxy_fcgi.c (revision 359901)
+++ modules/proxy/mod_proxy_fcgi.c (working copy)
@@ -351,11 +351,16 @@
return 0;
}
+typedef struct {
+apr_pool_t *scratch_pool;
+} proxy_fcgi_baton_t;
+
static apr_status_t dispatch(proxy_conn_rec *conn, request_rec *r,
int request_id)
{
apr_bucket_brigade *ib, *ob;
int seen_end_of_headers = 0, done = 0;
+proxy_fcgi_baton_t *pfb = conn->data;
apr_status_t rv = APR_SUCCESS;
conn_rec *c = r->connection;
struct iovec vec[2];
@@ -388,7 +393,7 @@
rv = ap_get_brigade(r->input_filters, ib,
AP_MODE_READBYTES, APR_BLOCK_READ,
-AP_IOBUFSIZE);
+sizeof(writebuf));
if (rv != APR_SUCCESS) {
break;
}
@@ -496,33 +501,29 @@
/* Clear out the header so our buffer is zeroed out again */
memset(readbuf, 0, 8);
-/* XXX We need support for content length > buffer size, but for
- * now just punt. */
+recv_again:
if ((clen + plen) > sizeof(readbuf) - 1) {
-ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
- "proxy: FCGI: back end server send more data "
- "than fits in buffer");
-rv = APR_EINVAL;
-break;
+readbuflen = sizeof(readbuf) - 1;
+} else {
+readbuflen = clen + plen;
}
/* Now get the actual data. Yes it sucks to do this in a second
* recv call, this will eventually change when we move to real
* nonblocking recv calls. */
-if ((clen + plen) != 0) {
-readbuflen = clen + plen;
-
+if (readbuflen != 0) {
rv = apr_socket_recv(conn->sock, readbuf, &readbuflen);
if (rv != APR_SUCCESS) {
break;
}
+readbuf[readbuflen] = 0;
}
switch (type) {
case FCGI_STDOUT:
if (clen != 0) {
b = apr_bucket_transient_create(readbuf,
-clen,
+readbuflen,
c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, b);
@@ -548,11 +549,31 @@
}
apr_brigade_cleanup(ob);
+
+apr_pool_clear(pfb->scratch_pool);
} else {
/* We're still looking for the end of the headers,
* so this part of the data will need to persist. */
-apr_bucket_setaside(b, r->pool);
+apr_bucket_setaside(b, pfb->scratch_pool);
}
+
+/* If we didn't read all the data go back and get the
+ * rest of it. */
+if ((clen + plen) > readbuflen) {
+if (! plen) {
+
