You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Sander Temme <sc...@apache.org> on 2008/08/15 07:04:10 UTC
[PATCH] Dynamic locking upcalls in mod_ssl
Folks,
The following patch against trunk adds dynamic locking callbacks to
mod_ssl. OpenSSL uses these in several places, including the CHIL
engine that interfaces with the nCipher products. I work at nCipher,
and this patch makes the CHIL engine load into a stock, unpatched
openssl 0.9.8.
I've tested this on Linux (Ubuntu 7.10 w/ OpensSSL 0.9.8e-5ubuntu3.2)
and put some load on this on Solaris 10 x86_64 with OpenSSL 0.9.8h.
It's not a lot of code, and I don't think it gets in anyone's way.
Opinions appreciated, especially on whether I'm doing the right thing
with that pool:
Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h (revision 686142)
+++ modules/ssl/ssl_private.h (working copy)
@@ -463,6 +463,16 @@
} SSLDirConfigRec;
/**
+ * Dynamic lock structure
+ */
+typedef struct {
+ apr_pool_t *pool;
+ const char* file;
+ int line;
+ apr_thread_mutex_t *mutex;
+} CRYPTO_dynlock_value;
+
+/**
* function prototypes
*/
Index: modules/ssl/ssl_engine_init.c
===================================================================
--- modules/ssl/ssl_engine_init.c (revision 686142)
+++ 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 686142)
+++ modules/ssl/ssl_util.c (working copy)
@@ -351,6 +351,106 @@
}
}
+/* Global reference to the pool used by the dynamic mutexes */
+apr_pool_t *dynlockpool;
+
+/*
+ * Dynamic lock creation callback
+ */
+static CRYPTO_dynlock_value *dyn_create_function(const char *file,
+ int line)
+{
+ CRYPTO_dynlock_value *value;
+ apr_pool_t *p;
+ apr_status_t r;
+
+ /*
+ * 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.
+ */
+ r = apr_pool_create(&p, dynlockpool);
+ if (r != APR_SUCCESS) {
+ ap_log_perror(file, line, APLOG_ERR, r, dynlockpool,
+ "Failed to create subpool for dynamic lock");
+ return NULL;
+ }
+
+ ap_log_perror(file, line, APLOG_DEBUG, 0, p,
+ "Creating dynamic lock");
+
+ value = (CRYPTO_dynlock_value *)apr_palloc(p,
+
sizeof(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;
+ r = apr_thread_mutex_create(&(value->mutex),
APR_THREAD_MUTEX_DEFAULT,
+ p);
+ if (r != APR_SUCCESS) {
+ ap_log_perror(file, line, APLOG_ERR, r, p,
+ "Failed to create thread mutex for dynamic
lock");
+ apr_pool_destroy(p);
+ return NULL;
+ }
+ return value;
+}
+
+/*
+ * Dynamic locking and unlocking function
+ */
+
+static void dyn_lock_function(int mode, CRYPTO_dynlock_value *l,
+ const char *file, int line)
+{
+ apr_status_t r;
+
+ if (mode & CRYPTO_LOCK) {
+ ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool,
+ "Acquiring mutex %s:%d", l->file, l->line);
+ r = apr_thread_mutex_lock(l->mutex);
+ ap_log_perror(file, line, APLOG_DEBUG, r, 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);
+ r = apr_thread_mutex_unlock(l->mutex);
+ ap_log_perror(file, line, APLOG_DEBUG, r, l->pool,
+ "Mutex %s:%d released!", l->file, l->line);
+ }
+}
+
+/*
+ * Dynamic lock destruction callback
+ */
+static void dyn_destroy_function(CRYPTO_dynlock_value *l,
+ const char *file, int line)
+{
+ apr_status_t r;
+
+ ap_log_perror(file, line, APLOG_DEBUG, 0, l->pool,
+ "Destroying dynamic lock %s:%d", l->file, l->line);
+ r = apr_thread_mutex_destroy(l->mutex);
+ if (r != APR_SUCCESS) {
+ ap_log_perror(file, line, APLOG_ERR, r, 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
@@ -393,6 +493,15 @@
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.
+ */
+ apr_pool_create(&dynlockpool, NULL);
+ apr_pool_tag(dynlockpool, "dynlockpool");
+ CRYPTO_set_dynlock_create_callback(dyn_create_function);
+ CRYPTO_set_dynlock_lock_callback(dyn_lock_function);
+ CRYPTO_set_dynlock_destroy_callback(dyn_destroy_function);
apr_pool_cleanup_register(p, NULL, ssl_util_thread_cleanup,
apr_pool_cleanup_null);
Thanks,
S.
--
Sander Temme
sctemme@apache.org
PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Sander Temme <sc...@apache.org>.
On Aug 20, 2008, at 11:56 AM, Plüm, Rüdiger, VF-Group wrote:
> 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.
Thanks Rüdiger, that's great feedback. With that, and a test run on
my Linux box where it responded successfully to a restart, a graceful
and a stop interspersed with ab runs that raised the number of worker
children, I'll commit. Where the whole thing will of course be up for
further review and consideration.
S.
--
Sander Temme
sctemme@apache.org
PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----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
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Aug 21, 2008 at 01:49:35PM +0200, "Plüm, Rüdiger, VF-Group" wrote:
> > Given that the lifetime of the callbacks is now constrained, is the
> > new global pool still needed?
>
> Where does this patch use a global pool? It keeps a reference on the pconf
> pool in a global variable, but it no longer creates a global pool.
> Or do I miss your point?
Oh, sorry, I missed that the dynlockpool is now parented off pconf.
Ignore me and carry on ;)
joe
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Joe Orton
> Gesendet: Donnerstag, 21. August 2008 13:41
> An: dev@httpd.apache.org
> Betreff: Re: [PATCH] Dynamic locking upcalls in mod_ssl
>
> On Wed, Aug 20, 2008 at 10:36:37AM -0400, Sander Temme wrote:
> >
> > 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?
>
> Given that the lifetime of the callbacks is now constrained,
> is the new
> global pool still needed?
Where does this patch use a global pool? It keeps a reference on the pconf
pool in a global variable, but it no longer creates a global pool.
Or do I miss your point?
Regards
Rüdiger
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Sander Temme <sc...@apache.org>.
On Aug 21, 2008, at 7:41 AM, Joe Orton wrote:
>> Yes, with the cleanup it no longer hangs. What about stashing a pool
>> reference in a global, is that a red flag?
>
> Given that the lifetime of the callbacks is now constrained, is the
> new
> global pool still needed?
As Rüdiger mentioned, the pool is no longer created out of a NULL
parent. Putting in the cleanup allowed me to use pconf without
hanging the parent on shutdown. It's still a global reference,
because the prototypes of the OpenSSL callbacks don't allow me to pass
user data back in.
> Also any reason to put the structure in ssl_private.h rather than
> the .c
> file directly?
None. I'll move it.
S.
--
Sander Temme
sctemme@apache.org
PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Aug 20, 2008 at 10:36:37AM -0400, Sander Temme wrote:
>
> 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?
Given that the lifetime of the callbacks is now constrained, is the new
global pool still needed?
Also any reason to put the structure in ssl_private.h rather than the .c
file directly?
joe
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Sander Temme <sc...@apache.org>.
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
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Aug 15, 2008 at 06:33:21AM -0700, Sander Temme wrote:
>
> On Aug 15, 2008, at 12:48 AM, Plüm, Rüdiger, VF-Group wrote:
>
>> 1. Why creating a global pool for dynlockpool? Why can't this be a
>> subpool
>> of the pool passed to ssl_util_thread_setup?
>
> Because that's the pconf pool and gets cleared across the lifetime of
> some of the mutexes that OpenSSL creates. I tried, and using pconf made
> the httpd parent hang on shutdown, in a tight loop trying to clear one of
> its pools...
mod_ssl needs to ensure that process-global callbacks registered with
OpenSSL will only last for the lifetime of the mod_ssl DSO. If those
function pointers remain stored by OpenSSL somewhere and are
dereferenced at a point where the mod_ssl DSO is no longer loaded (e.g.
during the startup ping-pong of DSOs), it's game over.
So generally pconf is the right pool to use, along with a cleanup
registered against that pool which sets the callbacks to NULL.
Regards, Joe
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Sander Temme <sc...@apache.org>.
On Aug 15, 2008, at 12:48 AM, Plüm, Rüdiger, VF-Group wrote:
> 1. Why creating a global pool for dynlockpool? Why can't this be a
> subpool
> of the pool passed to ssl_util_thread_setup?
Because that's the pconf pool and gets cleared across the lifetime of
some of the mutexes that OpenSSL creates. I tried, and using pconf
made the httpd parent hang on shutdown, in a tight loop trying to
clear one of its pools...
> 2. I would prefer to name the apr_status_t variables rv instead of r.
> r is normally reserved for the request :-).
Absolutely.
S.
--
Sander Temme
sctemme@apache.org
PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF
AW: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Sander Temme
> Gesendet: Freitag, 15. August 2008 07:04
> An: dev@httpd.apache.org
> Betreff: [PATCH] Dynamic locking upcalls in mod_ssl
>
> Folks,
>
> The following patch against trunk adds dynamic locking callbacks to
> mod_ssl. OpenSSL uses these in several places, including the CHIL
> engine that interfaces with the nCipher products. I work at
> nCipher,
> and this patch makes the CHIL engine load into a stock, unpatched
> openssl 0.9.8.
>
> I've tested this on Linux (Ubuntu 7.10 w/ OpensSSL
> 0.9.8e-5ubuntu3.2)
> and put some load on this on Solaris 10 x86_64 with OpenSSL 0.9.8h.
> It's not a lot of code, and I don't think it gets in anyone's way.
> Opinions appreciated, especially on whether I'm doing the
> right thing
> with that pool:
Two quick comments:
1. Why creating a global pool for dynlockpool? Why can't this be a subpool
of the pool passed to ssl_util_thread_setup?
2. I would prefer to name the apr_status_t variables rv instead of r.
r is normally reserved for the request :-).
Regards
Rüdiger
Re: [PATCH] Dynamic locking upcalls in mod_ssl
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Thu, Aug 14, 2008 at 10:04 PM, Sander Temme <sc...@apache.org> wrote:
> Folks,
>
> The following patch against trunk adds dynamic locking callbacks to mod_ssl.
> OpenSSL uses these in several places, including the CHIL engine that
> interfaces with the nCipher products. I work at nCipher, and this patch
> makes the CHIL engine load into a stock, unpatched openssl 0.9.8.
>
> I've tested this on Linux (Ubuntu 7.10 w/ OpensSSL 0.9.8e-5ubuntu3.2) and
> put some load on this on Solaris 10 x86_64 with OpenSSL 0.9.8h. It's not a
> lot of code, and I don't think it gets in anyone's way. Opinions
> appreciated, especially on whether I'm doing the right thing with that pool:
Going into the wayback machine a little bit, but it's sad how often we
need to keep putting this code everywhere. =)
http://svn.apache.org/repos/asf/httpd/flood/trunk/flood_net_ssl.c
http://code.google.com/p/serf/source/browse/trunk/buckets/ssl_buckets.c
I had serious flashbacks when I reviewed this patch. *grin* -- justin