thanks Ruediger, I see that you have a caught another instance of the
leak.
i'll give your patch a run and let you know the results in a day or two.
Regards,
Manik
On 23-Oct-08, at 12:31 AM, Ruediger Pluem wrote:
On 10/22/2008 05:33 PM, Jim Jagielski wrote:
On Oct 22, 2008, at 11:05 AM, Manik Taneja wrote:
With regards to the patch, it seems that the changes in event.c have
no effect. We have anyway seen the leak with both worker and event
mpm. The changes in scoreboard.c just ensure that memory pointed
to by
new_sbh is not lost everytime ap_create_sb_handle is called.
But this implies that any code which calls ap_create_sb_handle()
needs
to ensure that *new_sbh isn't NULL, and, iirc, most just define them
(eg: ap_sb_handle_t *sbh;) so the value is garbage. Thus, this seems
to also imply a change to the API (since current code will break
with the ap_create_sb_handle() change.
I have tested this fix and the memory consumption has dropped
drastically, however I still do observe a slow leak at times. I
haven't got the opportunity to spend a whole lot of time testing the
fix. Its on my plate however, and I'll send out my findings to the
group sometime next week.
Agreed that the mod_proxy_http.c patch looks reasonable at 1st
blush, but need to look into all possible interaction with the
change to a shorter-lived pool.
There is an explanation above the code why we should take connection-
>pool,
but IMHO it is outdated and goes back to the times where there was a
direct
connection between the backend connection and the frontend connection.
With the usage of the connection pools this is no longer the case
and I
cannot think of any reason why not using r->pool instead.
I used a slightly modified version of the patch:
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (Revision 707022)
+++ modules/proxy/mod_proxy_http.c (Arbeitskopie)
@@ -1869,23 +1869,9 @@
const char *u;
proxy_conn_rec *backend = NULL;
int is_ssl = 0;
-
- /* Note: Memory pool allocation.
- * A downstream keepalive connection is always connected to the
existence
- * (or not) of an upstream keepalive connection. If this is not
done then
- * load balancing against multiple backend servers breaks (one
backend
- * server ends up taking 100% of the load), and the risk is run
of
- * downstream keepalive connections being kept open
unnecessarily. This
- * keeps webservers busy and ties up resources.
- *
- * As a result, we allocate all sockets out of the upstream
connection
- * pool, and when we want to reuse a socket, we check first
whether the
- * connection ID of the current upstream connection is the same
as that
- * of the connection when the socket was opened.
- */
- apr_pool_t *p = r->connection->pool;
+ apr_pool_t *p = r->pool;
conn_rec *c = r->connection;
- apr_uri_t *uri = apr_palloc(r->connection->pool, sizeof(*uri));
+ apr_uri_t *uri = apr_palloc(p, sizeof(*uri));
/* find the scheme */
u = strchr(url, ':');
@@ -1893,7 +1879,7 @@
return DECLINED;
if ((u - url) > 14)
return HTTP_BAD_REQUEST;
- scheme = apr_pstrndup(c->pool, url, u - url);
+ scheme = apr_pstrndup(p, url, u - url);
/* scheme is lowercase */
ap_str_tolower(scheme);
/* is it for us? */
and it passes all perl framework tests.
Regards
Rüdiger
