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

named_atomic review

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Mon, 19 Nov 2012 19:24:55 +0200

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

This is an archived mail posted to the Subversion Dev mailing list.