A few questions about named_atomic.c:
Index: subversion/libsvn_subr/named_atomic.c
===================================================================
--- subversion/libsvn_subr/named_atomic.c (revision 1411269)
+++ subversion/libsvn_subr/named_atomic.c (working copy)
@@ -309,6 +309,7 @@ delete_lock_file(void *arg)
/* locks have already been cleaned up. Simply close the file */
apr_file_close(mutex->lock_file);
+ /* ### check the return value? */
/* Remove the file from disk. This will fail if there ares still other
* users of this lock file, i.e. namespace. */
@@ -324,6 +325,7 @@ delete_lock_file(void *arg)
static svn_error_t *
validate(svn_named_atomic__t *atomic)
{
+ /* ### What is the purpose of this validation? What does it do? */
return atomic && atomic->data && atomic->mutex
? SVN_NO_ERROR
: svn_error_create(SVN_ERR_BAD_ATOMIC, 0, _("Not a valid atomic"));
@@ -394,6 +396,7 @@ svn_atomic_namespace__create(svn_atomic_namespace_
const char *shm_name, *lock_name;
apr_finfo_t finfo;
+ /* ### Use a scratch_pool provided by caller? */
apr_pool_t *subpool = svn_pool_create(result_pool);
/* allocate the namespace data structure
@@ -421,10 +424,15 @@ svn_atomic_namespace__create(svn_atomic_namespace_
apr_pool_cleanup_register(result_pool, &new_ns->mutex,
delete_lock_file,
apr_pool_cleanup_null);
+ /* ### If svn_atomic_namespace__create() is called twice, the cleanup
+ ### handler will be installed twice - so the lock will be removed
+ ### as soon as _either_ if them is fired. Problem? */
/* Prevent concurrent initialization.
*/
SVN_ERR(lock(&new_ns->mutex));
+ /* ### What if two functions reach this line of code concurrently? Should
+ ### the rest of the function be an svn_atomic__init_once() callback? */
/* First, make sure that the underlying file exists. If it doesn't
* exist, create one and initialize its content.
Index: subversion/include/private/svn_named_atomic.h
===================================================================
--- subversion/include/private/svn_named_atomic.h (revision 1411269)
+++ subversion/include/private/svn_named_atomic.h (working copy)
@@ -104,8 +104,10 @@ svn_atomic_namespace__cleanup(const char *name,
* characters and an error will be returned if the specified name is longer
* than supported.
*
- * @note The lifetime of the atomic is bound to the lifetime
+ * @note The lifetime of the atomic object is bound to the lifetime
* of the @a ns object, i.e. the pool the latter was created in.
+ * The namespace persists as long as at least one process holds
+ * an #svn_atomic_namespace__t object corresponding to it.
*/
svn_error_t *
svn_named_atomic__get(svn_named_atomic__t **atomic,
I've skipped some details, but those that I haven't appear to be
straightforward.
Cheers
Daniel
Received on 2012-11-19 18:25:34 CET