httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sander Temme <scte...@apache.org>
Subject Re: [PATCH] Dynamic locking upcalls in mod_ssl
Date Wed, 20 Aug 2008 14:36:37 GMT

On Aug 18, 2008, at 5:18 AM, Joe Orton wrote:

> So generally pconf is the right pool to use, along with a cleanup
> registered against that pool which sets the callbacks to NULL.

Yes, with the cleanup it no longer hangs.  What about stashing a pool  
reference in a global, is that a red flag?

The patch now looks something like this, with the rv change RĂ¼diger  
suggested and some tweaks to get the types in line and make the  
compiler shut up:

Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h	(revision 687227)
+++ modules/ssl/ssl_private.h	(working copy)
@@ -463,6 +463,16 @@
  } SSLDirConfigRec;

  /**
+ * Dynamic lock structure
+ */
+struct CRYPTO_dynlock_value {
+    apr_pool_t *pool;
+    const char* file;
+    int line;
+    apr_thread_mutex_t *mutex;
+};
+
+/**
   *  function prototypes
   */

Index: modules/ssl/ssl_engine_init.c
===================================================================
--- modules/ssl/ssl_engine_init.c	(revision 687227)
+++ modules/ssl/ssl_engine_init.c	(working copy)
@@ -321,6 +321,9 @@
              ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, s);
              ssl_die();
          }
+        ap_log_error(APLOG_MARK, APLOG_INFO, 0, s,
+                     "Init: loaded Crypto Device API `%s'",
+                     mc->szCryptoDevice);

          ENGINE_free(e);
      }
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;
+
+/*
+ * 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);
+    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);

      /* Let the registered mutex cleanups do their own thing
       */
@@ -393,6 +497,14 @@
      CRYPTO_set_id_callback(ssl_util_thr_id);

      CRYPTO_set_locking_callback(ssl_util_thr_lock);
+
+    /* Set up dynamic locking scaffolding for OpenSSL to use at its
+     * convenience.
+     */
+    dynlockpool = p;
+    CRYPTO_set_dynlock_create_callback(ssl_dyn_create_function);
+    CRYPTO_set_dynlock_lock_callback(ssl_dyn_lock_function);
+    CRYPTO_set_dynlock_destroy_callback(ssl_dyn_destroy_function);

      apr_pool_cleanup_register(p, NULL, ssl_util_thread_cleanup,
                                         apr_pool_cleanup_null);


S.

-- 
Sander Temme
sctemme@apache.org
PGP FP: 51B4 8727 466A 0BC3 69F4  B7B8 B2BE BC40 1529 24AF


Mime
View raw message