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.
> +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.
I attached the new patch.
--
Vlad
---------------------------------------------------------------------
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:30:46 2006