git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: svn commit: r1198930 - in /httpd/httpd/trunk: include/mod_core.h server/core.c server/main.c



On 11/07/2011 09:57 PM, sf@xxxxxxxxxx wrote:
> Author: sf
> Date: Mon Nov  7 20:57:02 2011
> New Revision: 1198930
> 

> 
> Modified: httpd/httpd/trunk/server/main.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/main.c?rev=1198930&r1=1198929&r2=1198930&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/main.c (original)
> +++ httpd/httpd/trunk/server/main.c Mon Nov  7 20:57:02 2011
> @@ -34,6 +34,7 @@
>  #include "http_log.h"
>  #include "http_config.h"
>  #include "http_core.h"
> +#include "mod_core.h"
>  #include "http_request.h"
>  #include "http_vhost.h"
>  #include "apr_uri.h"
> @@ -459,6 +460,7 @@ int main(int argc, const char * const ar
>      ap_pglobal = process->pool;
>      pconf = process->pconf;
>      ap_server_argv0 = process->short_name;
> +    ap_init_rng(ap_pglobal);

With APR trunk used this now causes httpd to SEGFAULT in EVP_cleanup when it stops in case mod_ssl is loaded.
This is because mod_ssl stored data in Openssl data structures that points to it (likely static data in mod_ssl), but it
gets unloaded due to the pconf pool cleanup before the crypto_lib_cleanup runs EVP_cleanup as it is a cleanup on the
parent pool ap_pglobal.
One approach that made this go away was the following patch to APR:

Index: srclib/apr/crypto/apr_crypto.c
===================================================================
--- srclib/apr/crypto/apr_crypto.c      (revision 1837332)
+++ srclib/apr/crypto/apr_crypto.c      (working copy)
@@ -534,8 +534,7 @@
         lib->pool = pool;
         apr_hash_set(active_libs, lib->name, APR_HASH_KEY_STRING, lib);
         if (apr_pool_parent_get(pool)) {
-            apr_pool_cleanup_register(pool, lib, crypto_lib_cleanup,
-                                      apr_pool_cleanup_null);
+            apr_pool_pre_cleanup_register(pool, lib, crypto_lib_cleanup);
         }
     }
     else {

So using a pre_cleanup instead of a cleanup. But I guess this would only fix this specific use case. I guess we have
a larger problem lurking around that a shared object can put some data in Openssl that points to it (e.g. static data in
the shared object) and that it is gone by the time the pool cleanup runs. In this case we are lucky that a child pool
causes the shared object to be unloaded and hence the pre_cleanup works here. But IMHO we don't need to have this
connection always. But I guess that discussion is more for dev@apr. Feel free to cross post.


Regards

Rüdiger