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

Re: [PATCH] svn_dso_initialize can *silently* fail to create lock

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 1 Aug 2008 19:20:17 +0200

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

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.