[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Client-side Cyrus SASL support

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2006-08-09 22:30:30 CEST

On 8/9/06, Vlad Georgescu <vgeorgescu@gmail.com> wrote:
> On 8/9/06, Garrett Rooney <rooneg@electricjellyfish.net> wrote:
> > A few more problems:
> >
> > +/* Define sane defaults for a sasl_security_properties_t structure.
> > + See sasl.h for details. SASL needs to know our read buffer's size
> > + when negotiating a security layer. */
> > +#define SVN_RA_SVN__DEFAULT_SECPROPS {0, 256, SVN_RA_SVN__READBUF_SIZE, \
> > + 0, NULL, NULL}
> >
> > Please explain what 256 means in this context. I think it's been
> > discussed on this list, but it really should make it into the comment.
>
> Done.

Thanks!

> > +static void *sasl_mutex_alloc_cb(void)
> > +{
> > + apr_thread_mutex_t *mutex;
> > + apr_status_t apr_err;
> > + if (apr_is_empty_array(free_mutexes))
> > + {
> > + apr_err = apr_thread_mutex_create(&mutex,
> > + APR_THREAD_MUTEX_DEFAULT,
> > + sasl_pool);
> > + if (apr_err != APR_SUCCESS)
> > + return NULL;
> > + }
> > + else
> > + {
> > + apr_err = apr_thread_mutex_lock(array_mutex);
> > + if (apr_err != APR_SUCCESS)
> > + return NULL;
> > + mutex = *((apr_thread_mutex_t**)apr_array_pop(free_mutexes));
> > + apr_err = apr_thread_mutex_unlock(array_mutex);
> > + if (apr_err != APR_SUCCESS)
> > + return NULL;
> > + }
> > + return mutex;
> > +}
> >
> > You can't access the array without locking the mutex. The lock needs
> > to be pulled up outside the if statement, before you call
> > apr_is_empty_array, otherwise none of it is thread safe. Basically
> > you need to lock the mutex before ANY access to that array.
>
> Oops. Thanks for pointing that out.

Still not quite right:

+static void *sasl_mutex_alloc_cb(void)
+{
+ apr_thread_mutex_t *mutex;
+ apr_status_t apr_err;
+
+ apr_err = apr_thread_mutex_lock(array_mutex);
+ if (apr_err != APR_SUCCESS)
+ return NULL;
+
+ if (apr_is_empty_array(free_mutexes))
+ {
+ apr_err = apr_thread_mutex_create(&mutex,
+ APR_THREAD_MUTEX_DEFAULT,
+ sasl_pool);
+ if (apr_err != APR_SUCCESS)
+ return NULL;

If you return here you'll leave the mutex locked, you've gotta unlock
it before returning.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 9 22:32:25 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.