You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2015/03/10 15:01:12 UTC
svn commit: r1665554 - in /subversion/trunk/subversion:
include/private/svn_atomic.h libsvn_subr/atomic.c libsvn_subr/error.c
Author: brane
Date: Tue Mar 10 14:01:12 2015
New Revision: 1665554
URL: http://svn.apache.org/r1665554
Log:
Make the atomic initialization implementation more robust
in the presence of wrong initial or intermediate values of
the control variable.
Also create a vairant that does not use a pool or create
an svn_error_t, so that it can be safely used during
initialization of the error system itself.
* subversion/include/private/svn_atomic.h
(svn_atomic__err_init_func_t, svn_atomic__str_init_func_t): New.
Typedefs for atomic initialization callback functions.
(svn_atomic__init_once): Use svn_atomic__err_init_func_t in the
prototype and update the docstring.
(svn_atomic__init_once_no_error): New.
* subversion/libsvn_subr/atomic.c: Include assert.h.
(init_once): New. Common implementation of both atomic-init functions.
(svn_atomic__init_once, svn_atomic__init_once_no_error):
Implement these with appropriate calls to init_once.
* subversion/libsvn_subr/error.c
(locate_init_once): Convert to an svn_atomic__str_init_func_t.
(svn_error__locate): Use svn_atomic__init_once_no_error.
Modified:
subversion/trunk/subversion/include/private/svn_atomic.h
subversion/trunk/subversion/libsvn_subr/atomic.c
subversion/trunk/subversion/libsvn_subr/error.c
Modified: subversion/trunk/subversion/include/private/svn_atomic.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_atomic.h?rev=1665554&r1=1665553&r2=1665554&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_atomic.h (original)
+++ subversion/trunk/subversion/include/private/svn_atomic.h Tue Mar 10 14:01:12 2015
@@ -76,21 +76,66 @@ extern "C" {
/** @} */
/**
+ * @name Single-threaded atomic initialization
+ * @{
+ */
+
+/**
+ * Callback for svn_atomic__init_once().
+ * @return an #svn_error_t if the initialization fails.
+ * @since New in 1.10
+ */
+typedef svn_error_t *(svn_atomic__err_init_func_t)(void *baton,
+ apr_pool_t *pool);
+
+/**
+ * Callback for svn_atomic__init_no_error().
+ * @return a string containing an error message if the initialization fails.
+ * @since New in 1.10
+ */
+typedef const char *(svn_atomic__str_init_func_t)(void *baton);
+
+/**
* Call an initialization function in a thread-safe manner.
*
* @a global_status must be a pointer to a global, zero-initialized
- * #svn_atomic_t. @a init_func is a pointer to the function that performs
- * the actual initialization. @a baton and and @a pool are passed on to the
- * init_func for its use.
+ * #svn_atomic_t. @a err_init_func is a pointer to the function that
+ * performs the actual initialization. @a baton and and @a pool are
+ * passed on to @a err_init_func for its use.
+ *
+ * @return the error returned by @a err_init_func.
*
* @since New in 1.5.
*/
svn_error_t *
svn_atomic__init_once(volatile svn_atomic_t *global_status,
- svn_error_t *(*init_func)(void*,apr_pool_t*),
+ svn_atomic__err_init_func_t err_init_func,
void *baton,
apr_pool_t* pool);
+/**
+ * Call an initialization function in a thread-safe manner.
+ *
+ * Unlike svn_atomic__init_once(), this function does not need a pool
+ * and does not create an #svn_error_t, and neither should the
+ * @a str_init_func implementation.
+ *
+ * @a global_status must be a pointer to a global, zero-initialized
+ * #svn_atomic_t. @a str_init_func is a pointer to the function that
+ * performs the actual initialization. @a baton is passed on to
+ * @a str_init_func for its use.
+ *
+ * @return the error string returned by @a str_init_func.
+ *
+ * @since New in 1.10.
+ */
+const char *
+svn_atomic__init_once_no_error(volatile svn_atomic_t *global_status,
+ svn_atomic__str_init_func_t str_init_func,
+ void *baton);
+
+/** @} */
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Modified: subversion/trunk/subversion/libsvn_subr/atomic.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/atomic.c?rev=1665554&r1=1665553&r2=1665554&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/atomic.c (original)
+++ subversion/trunk/subversion/libsvn_subr/atomic.c Tue Mar 10 14:01:12 2015
@@ -20,6 +20,7 @@
* ====================================================================
*/
+#include <assert.h>
#include <apr_time.h>
#include "private/svn_atomic.h"
@@ -29,11 +30,20 @@
#define SVN_ATOMIC_INIT_FAILED 2
#define SVN_ATOMIC_INITIALIZED 3
-svn_error_t*
-svn_atomic__init_once(volatile svn_atomic_t *global_status,
- svn_error_t *(*init_func)(void*,apr_pool_t*),
- void *baton,
- apr_pool_t* pool)
+
+/*
+ * This is the actual atomic initialization driver. The caller must
+ * provide either ERR_INIT_FUNC and ERR_P, or STR_INIT_FUNC and
+ * ERRSTR_P, but never both.
+ */
+static void
+init_once(svn_atomic__err_init_func_t err_init_func,
+ svn_error_t **err_p,
+ svn_atomic__str_init_func_t str_init_func,
+ const char **errstr_p,
+ volatile svn_atomic_t *global_status,
+ void *baton,
+ apr_pool_t* pool)
{
/* !! Don't use localizable strings in this function, because these
!! might cause deadlocks. This function can be used to initialize
@@ -42,44 +52,94 @@ svn_atomic__init_once(volatile svn_atomi
/* We have to call init_func exactly once. Because APR
doesn't have statically-initialized mutexes, we implement a poor
man's spinlock using svn_atomic_cas. */
+
+ svn_error_t *err = SVN_NO_ERROR;
+ const char *errstr = NULL;
svn_atomic_t status = svn_atomic_cas(global_status,
SVN_ATOMIC_START_INIT,
SVN_ATOMIC_UNINITIALIZED);
- if (status == SVN_ATOMIC_UNINITIALIZED)
+#ifdef SVN_DEBUG
+ /* Check that the parameters are valid. */
+ assert(!err_init_func != !str_init_func);
+ assert(!err_init_func == !err_p);
+ assert(!str_init_func == !errstr_p);
+#endif /* SVN_DEBUG */
+
+ for (;;)
{
- svn_error_t *err = init_func(baton, pool);
- if (err)
+ switch (status)
{
-#if APR_HAS_THREADS
- /* Tell other threads that the initialization failed. */
- svn_atomic_cas(global_status,
- SVN_ATOMIC_INIT_FAILED,
- SVN_ATOMIC_START_INIT);
-#endif
- return svn_error_create(SVN_ERR_ATOMIC_INIT_FAILURE, err,
- "Couldn't perform atomic initialization");
+ case SVN_ATOMIC_UNINITIALIZED:
+ if (err_init_func)
+ err = err_init_func(baton, pool);
+ else
+ errstr = str_init_func(baton);
+ if (err || errstr)
+ {
+ status = svn_atomic_cas(global_status,
+ SVN_ATOMIC_INIT_FAILED,
+ SVN_ATOMIC_START_INIT);
+ }
+ else
+ {
+ status = svn_atomic_cas(global_status,
+ SVN_ATOMIC_INITIALIZED,
+ SVN_ATOMIC_START_INIT);
+ }
+
+ /* Take another spin through the switch to report either
+ failure or success. */
+ continue;
+
+ case SVN_ATOMIC_START_INIT:
+ /* Wait for the init function to complete. */
+ apr_sleep(APR_USEC_PER_SEC / 1000);
+ status = svn_atomic_cas(global_status,
+ SVN_ATOMIC_UNINITIALIZED,
+ SVN_ATOMIC_UNINITIALIZED);
+ continue;
+
+ case SVN_ATOMIC_INIT_FAILED:
+ if (err_init_func)
+ *err_p = svn_error_create(SVN_ERR_ATOMIC_INIT_FAILURE, err,
+ "Couldn't perform atomic initialization");
+ else
+ *errstr_p = errstr;
+ return;
+
+ case SVN_ATOMIC_INITIALIZED:
+ if (err_init_func)
+ *err_p = SVN_NO_ERROR;
+ else
+ *errstr_p = NULL;
+ return;
+
+ default:
+ /* Something went seriously wrong with the atomic operations. */
+ abort();
}
- svn_atomic_cas(global_status,
- SVN_ATOMIC_INITIALIZED,
- SVN_ATOMIC_START_INIT);
- }
-#if APR_HAS_THREADS
- /* Wait for whichever thread is performing initialization to finish. */
- /* XXX FIXME: Should we have a maximum wait here, like we have in
- the Windows file IO spinner? */
- else while (status != SVN_ATOMIC_INITIALIZED)
- {
- if (status == SVN_ATOMIC_INIT_FAILED)
- return svn_error_create(SVN_ERR_ATOMIC_INIT_FAILURE, NULL,
- "Couldn't perform atomic initialization");
-
- apr_sleep(APR_USEC_PER_SEC / 1000);
- status = svn_atomic_cas(global_status,
- SVN_ATOMIC_UNINITIALIZED,
- SVN_ATOMIC_UNINITIALIZED);
}
-#endif /* APR_HAS_THREADS */
+}
+
- return SVN_NO_ERROR;
+svn_error_t *
+svn_atomic__init_once(volatile svn_atomic_t *global_status,
+ svn_atomic__err_init_func_t err_init_func,
+ void *baton,
+ apr_pool_t* pool)
+{
+ svn_error_t *err;
+ init_once(err_init_func, &err, NULL, NULL, global_status, baton, pool);
+ return err;
+}
+
+const char *
+svn_atomic__init_once_no_error(volatile svn_atomic_t *global_status,
+ svn_atomic__str_init_func_t str_init_func,
+ void *baton)
+{
+ const char *errstr;
+ init_once(NULL, NULL, str_init_func, &errstr, global_status, baton, NULL);
+ return errstr;
}
Modified: subversion/trunk/subversion/libsvn_subr/error.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/error.c?rev=1665554&r1=1665553&r2=1665554&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/error.c (original)
+++ subversion/trunk/subversion/libsvn_subr/error.c Tue Mar 10 14:01:12 2015
@@ -59,11 +59,12 @@ static apr_threadkey_t *error_line_key =
/* No-op destructor for apr_threadkey_private_create(). */
static void null_threadkey_dtor(void *stuff) {}
-/* Handler for svn_atomic__init_once used by svn_error__locate to
- initialize the thread-local error location storage.
- This function will never return an error. */
-static svn_error_t *
-locate_init_once(void *ignored_baton, apr_pool_t *ignored_pool)
+/* Implements svn_atomic__str_init_func_t.
+ Callback used by svn_error__locate to initialize the thread-local
+ error location storage. This function will never return an
+ error string. */
+static const char *
+locate_init_once(void *ignored_baton)
{
/* Strictly speaking, this is a memory leak, since we're creating an
unmanaged, top-level pool and never destroying it. We do this
@@ -86,7 +87,7 @@ locate_init_once(void *ignored_baton, ap
if (status != APR_SUCCESS)
error_file_key = error_line_key = NULL;
- return SVN_NO_ERROR;
+ return NULL;
}
# endif /* APR_HAS_THREADS */
@@ -124,9 +125,7 @@ svn_error__locate(const char *file, long
#ifdef SVN_DEBUG
# if APR_HAS_THREADS
static volatile svn_atomic_t init_status = 0;
- svn_error_clear(svn_atomic__init_once(&init_status,
- locate_init_once,
- NULL, NULL));
+ svn_atomic__init_once_no_error(&init_status, locate_init_once, NULL);
if (error_file_key && error_line_key)
{