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:[EMAIL PROTECTED] >Sent: Wednesday, March 24, 2004 7:49 AM >To: [EMAIL PROTECTED] >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; > } > > /* >
