httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Plüm, Rüdiger, VF-Group" <ruediger.pl...@vodafone.com>
Subject Re: [PATCH] Dynamic locking upcalls in mod_ssl
Date Wed, 20 Aug 2008 15:56:05 GMT
 

> -----Ursprüngliche Nachricht-----
> Von: Sander Temme 
> Gesendet: Mittwoch, 20. August 2008 16:37
> An: dev@httpd.apache.org
> Cc: Joe Orton
> Betreff: Re: [PATCH] Dynamic locking upcalls in mod_ssl
> 
> 
> Index: modules/ssl/ssl_util.c
> ===================================================================
> --- modules/ssl/ssl_util.c	(revision 687227)
> +++ modules/ssl/ssl_util.c	(working copy)
> @@ -351,6 +351,106 @@
>       }
>   }
> 
> +/* Global reference to the pool passed into 
> ssl_util_thread_setup() */
> +apr_pool_t *dynlockpool;

Should be initialized with NULL.

> +
> +/*
> + * Dynamic lock creation callback
> + */
> +static struct CRYPTO_dynlock_value *ssl_dyn_create_function(const  
> char *file,
> +                                                     int line)
> +{
> +    struct CRYPTO_dynlock_value *value;
> +    apr_pool_t *p;
> +    apr_status_t rv;
> +
> +    /*
> +     * We need a pool to allocate our mutex.  Since we can't clear
> +     * allocated memory from a pool, create a subpool that 
> we can blow
> +     * away in the destruction callback.
> +     */
> +    rv = apr_pool_create(&p, dynlockpool);
> +    if (rv != APR_SUCCESS) {
> +        ap_log_perror(file, line, APLOG_ERR, rv, dynlockpool,
> +                       "Failed to create subpool for dynamic lock");
> +        return NULL;
> +    }
> +
> +    ap_log_perror(file, line, APLOG_DEBUG, 0, p,
> +                  "Creating dynamic lock");
> +
> +    value = (struct CRYPTO_dynlock_value *)apr_palloc(p,
> +                                                      sizeof(struct  
> CRYPTO_dynlock_value));
> +    if (!value) {
> +        ap_log_perror(file, line, APLOG_ERR, 0, p,
> +                      "Failed to allocate dynamic lock structure");
> +        return NULL;
> +    }
> +
> +    value->pool = p;
> +    /* Keep our own copy of the place from which we were created,
> +       using our own pool. */
> +    value->file = apr_psprintf(p, "%s", file);

Hm, why not doing an apr_pstrdup here? This seems to be more efficient
than an apr_psprintf in this situation.

> +    value->line = line;
> +    rv = apr_thread_mutex_create(&(value->mutex),  
> APR_THREAD_MUTEX_DEFAULT,
> +                                p);
> +    if (rv != APR_SUCCESS) {
> +        ap_log_perror(file, line, APLOG_ERR, rv, p,
> +                      "Failed to create thread mutex for dynamic  
> lock");
> +        apr_pool_destroy(p);
> +        return NULL;
> +    }
> +    return value;
> +}
> +
> +/*
> + * Dynamic locking and unlocking function
> + */
> +
> +static void ssl_dyn_lock_function(int mode, struct  
> CRYPTO_dynlock_value *l,
> +                           const char *file, int line)
> +{
> +    apr_status_t rv;
> +
> +    if (mode & CRYPTO_LOCK) {
> +        ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool,
> +                      "Acquiring mutex %s:%d", l->file, l->line);
> +        rv = apr_thread_mutex_lock(l->mutex);
> +        ap_log_perror(file, line, APLOG_DEBUG, rv, l->pool,
> +                      "Mutex %s:%d acquired!", l->file, l->line);
> +    }
> +    else {
> +        ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool,
> +                      "Releasing mutex %s:%d", l->file, l->line);
> +        rv = apr_thread_mutex_unlock(l->mutex);
> +        ap_log_perror(file, line, APLOG_DEBUG, rv, l->pool,
> +                      "Mutex %s:%d released!", l->file, l->line);
> +    }
> +}
> +
> +/*
> + * Dynamic lock destruction callback
> + */
> +static void ssl_dyn_destroy_function(struct CRYPTO_dynlock_value *l,
> +                          const char *file, int line)
> +{
> +    apr_status_t rv;
> +
> +    ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool,
> +                  "Destroying dynamic lock %s:%d", l->file, l->line);
> +    rv = apr_thread_mutex_destroy(l->mutex);
> +    if (rv != APR_SUCCESS) {
> +        ap_log_perror(file, line, APLOG_ERR, rv, l->pool,
> +                      "Failed to destroy mutex for dynamic 
> lock %s:%d",
> +                      l->file, l->line);
> +    }
> +
> +    /* Trust that whomever owned the CRYPTO_dynlock_value we were
> +     * passed has no future use for it...
> +     */
> +    apr_pool_destroy(l->pool);
> +}
> +
>   static unsigned long ssl_util_thr_id(void)
>   {
>       /* OpenSSL needs this to return an unsigned long.  On OS/390,  
> the pthread
> @@ -373,6 +473,10 @@
>   {
>       CRYPTO_set_locking_callback(NULL);
>       CRYPTO_set_id_callback(NULL);
> +
> +    CRYPTO_set_dynlock_create_callback(NULL);
> +    CRYPTO_set_dynlock_lock_callback(NULL);
> +    CRYPTO_set_dynlock_destroy_callback(NULL);

You should set dynlockpool to NULL here as well. In case it is used afterwards
things segfault and are easier to detected than when an invalid pointer is used.
This should basicly address your question regarding the reference on a pool
in global variable as well.

Regards

Rüdiger


Mime
View raw message