On Fri, Aug 01, 2008 at 08:58:57PM +0530, Kamesh Jayachandran wrote:
> Hi All,
>
> The svn_dso_initialize can *silently* fail to create lock causing
> failures later.
>
> Attached patch fixes it.
>
> Want to know what others would think about it.
Looks good to me.
A curiosity: The return value of apr_thread_mutex_create is
not checked consistently even within APR itself.
Have you seen crashes because of this
In apr-1.3.2, the function can only return an error it if fails
to allocate memory (at least the way we're calling it with
APR_THREAD_MUTEX_DEFAULT). Anyway, checking the error code is a good
idea. We don't want to rely on implementation details like that.
I would suggest using
SVN_ERR_ASSERT(status == APR_SUCCESS);
instead of
if (status)
abort();
to give clients a chance to recover from the error.
I think this is one of those cases we introduced SVN_ERR_ASSERT for.
Stefan
> Fix 'svn_dso_initialize' which was *silently* failing to create lock
> and causing later accessors to fail.
>
> * subversion/libsvn_subr/dso.c
> (svn_dso_initialize): When 'apr_thread_mutex_create' is not successful,
> abort.
> Index: subversion/libsvn_subr/dso.c
> ===================================================================
> --- subversion/libsvn_subr/dso.c (revision 32351)
> +++ subversion/libsvn_subr/dso.c (working copy)
> @@ -44,13 +44,17 @@
> void
> svn_dso_initialize()
> {
> + apr_status_t status;
> if (dso_pool)
> return;
>
> dso_pool = svn_pool_create(NULL);
>
> #if APR_HAS_THREADS
> - apr_thread_mutex_create(&dso_mutex, APR_THREAD_MUTEX_DEFAULT, dso_pool);
> + status = apr_thread_mutex_create(&dso_mutex,
> + APR_THREAD_MUTEX_DEFAULT, dso_pool);
> + if (status)
> + abort();
> #endif
>
> dso_cache = apr_hash_make(dso_pool);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-01 19:20:38 CEST