httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mathihalli, Madhusudan" <mad...@hp.com>
Subject RE: [PATCH ?] RE: SEGV in allocator_free
Date Wed, 24 Mar 2004 18:37:49 GMT
Yes. I tested something similar (just don't register the cleanup function) - and things seemed
to work without any SEGV. I had the same question as yours : if a ssl_filter_io_shutdown()
was already done by EOC flag, the filter_cleanup should be a noop - why should it fail ?

Anyways, it appears to me that we don't really need the cleanup. I don't mind adjusting to
what you proposed (try to be good citizen and cleanup whenever/where-ever possible). I'll
test it right away !

-Madhu

>-----Original Message-----
>From: Joe Orton [mailto:jorton@redhat.com]
>Sent: Wednesday, March 24, 2004 7:49 AM
>To: dev@httpd.apache.org
>Subject: Re: [PATCH ?] RE: SEGV in allocator_free
>
>
>On Fri, Mar 19, 2004 at 06:51:41PM -0800, Mathihalli, Madhusudan wrote:
>> Do we need to do the following ? I tried it - the test continued to a
>> certain extent, only to fail again after some time (with the same
>> stack trace)
>
>What's the repro case for this? You're running swamp against an
>SSL->HTTP reverse proxy?  The problem seems to be because:
>
>- ssl_io_filter_cleanup() is registered as a cleanup on ptrans.
>
>- core_output_filter creates the deferred_write_pool as a subpool of
>ptrans.
>
>- when ssl_io_filter_cleanup() runs, it calls ssl_filter_io_shutdown(),
>which tries to do an SSL shutdown and hence passes a brigade down
>through core_output_filter.
>
>- apr_pool_clear destroys subpools before running cleanups
>
>So when apr_pool_clear(ptrans) is run in the MPM, it goes:
>
>1. destroy deferred_write_pool
>2. run ssl_io_filter_cleanup, which calls core_output_filter
>3. core_output_filter goes boom since its ->deferred_write_pool pointer
>is pointing at a destroyed pool
>
>I'm not sure why this would trigger in 2.0.49 but not .48 
>particularly. 
>If anything I'd expect the opposite because the EOC should ensure that
>ssl_filter_io_shutdown is a noop when called from the pool 
>cleanup under
>normal operation.  Hmmm.
>
>I think the correct fix is to stop trying to send the shutdown from the
>cleanup, which didn't actually work anyway.  Can you test something
>like:
>
>Index: ssl_engine_io.c
>===================================================================
>RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v
>retrieving revision 1.121
>diff -u -r1.121 ssl_engine_io.c
>--- ssl_engine_io.c	29 Feb 2004 00:29:20 -0000	1.121
>+++ ssl_engine_io.c	24 Mar 2004 15:44:37 -0000
>@@ -984,22 +984,19 @@
> 
> static apr_status_t ssl_io_filter_cleanup(void *data)
> {
>-    apr_status_t ret;
>-    ssl_filter_ctx_t *filter_ctx = (ssl_filter_ctx_t *)data;
>-    conn_rec *c;
>+    ssl_filter_ctx_t *filter_ctx = data;
> 
>-    if (!filter_ctx->pssl) {
>-        /* already been shutdown */
>-        return APR_SUCCESS;
>-    }
>+    if (filter_ctx->pssl) {
>+        conn_rec *c = (conn_rec *)SSL_get_app_data(filter_ctx->pssl);
>+        SSLConnRec *sslconn = myConnConfig(c);
> 
>-    c = (conn_rec *)SSL_get_app_data(filter_ctx->pssl);
>-    if ((ret = ssl_filter_io_shutdown(filter_ctx, c, 0)) != 
>APR_SUCCESS) {
>-        ap_log_error(APLOG_MARK, APLOG_INFO, ret, NULL,
>-                     "SSL filter error shutting down I/O");
>+        SSL_free(filter_ctx->pssl);
>+        filter_ctx->pssl = NULL;
>+        
>+        if (sslconn) sslconn->ssl = NULL;
>     }
> 
>-    return ret;
>+    return APR_SUCCESS;
> }
> 
> /*
>

Mime
View raw message