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
[EMAIL PROTECTED]
PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF
mod_ssl_dynamic_lock.patch
Description: Binary data
smime.p7s
Description: S/MIME cryptographic signature
