svn commit: r1537357 - /subversion/trunk/subversion/libsvn_repos/config_pool.c

[st...@apache.org]

Author: stefan2
Date: Thu Oct 31 00:34:25 2013
New Revision: 1537357

URL: http://svn.apache.org/r1537357
Log:
Fix various issues with the config_pool object that came up during testing.

* subversion/libsvn_repos/config_pool.c
  (config_pool_cleanup,
   config_ref_cleanup): make sure only one thread acquires the cleanup token
  (config_by_url): use our specified hash function
  (svn_repos__config_pool_create): allocate in the right pool
  (svn_repos__config_pool_get): check the right pointer

Modified:
    subversion/trunk/subversion/libsvn_repos/config_pool.c

Modified: subversion/trunk/subversion/libsvn_repos/config_pool.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/config_pool.c?rev=1537357&r1=1537356&r2=1537357&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/config_pool.c (original)
+++ subversion/trunk/subversion/libsvn_repos/config_pool.c Thu Oct 31 00:34:25 2013
@@ -29,6 +29,7 @@
 #include "svn_checksum.h"
 #include "svn_config.h"
 #include "svn_error.h"
+#include "svn_hash.h"
 #include "svn_path.h"
 #include "svn_pools.h"
 #include "svn_repos.h"
@@ -176,10 +177,14 @@ config_pool_cleanup(void *baton)
 {
   svn_repos__config_pool_t *config_pool = baton;
 
+  /* from now on, anyone is allowed to destroy the config_pool */
   svn_atomic_set(&config_pool->ready_for_cleanup, TRUE);
-  if (svn_atomic_read(&config_pool->used_config_count) == 0)
+
+  /* are there no more external references and can we grab the cleanup flag? */
+  if (   svn_atomic_read(&config_pool->used_config_count) == 0
+      && svn_atomic_cas(&config_pool->ready_for_cleanup, FALSE, TRUE) == TRUE)
     {
-      /* Attempts to get a configuration from a pool that whose cleanup has
+      /* Attempts to get a configuration from a pool whose cleanup has
        * already started is illegal.
        * So, used_config_count must not increase again.
        */
@@ -198,9 +203,9 @@ config_ref_cleanup(void *baton)
   svn_repos__config_pool_t *config_pool = config->config_pool;
 
   /* Maintain reference counters and handle object cleanup */
-  if (   svn_atomic_dec(&config->ref_count) == 1
-      && svn_atomic_dec(&config_pool->used_config_count) == 1
-      && svn_atomic_read(&config_pool->ready_for_cleanup) == TRUE)
+  if (   svn_atomic_dec(&config->ref_count) == 0
+      && svn_atomic_dec(&config_pool->used_config_count) == 0
+      && svn_atomic_cas(&config_pool->ready_for_cleanup, FALSE, TRUE) == TRUE)
     {
       /* There cannot be any future references to a config in this pool.
        * So, we are the last one and need to finally clean it up.
@@ -396,7 +401,7 @@ add_checksum(svn_repos__config_pool_t *c
   apr_size_t path_len = strlen(url);
   apr_pool_t *pool = config_pool->in_repo_hash_pool;
   in_repo_config_t *config = apr_hash_get(config_pool->in_repo_configs,
-                                            url, path_len);
+                                          url, path_len);
   if (config)
     {
       /* update the existing entry */
@@ -535,8 +540,7 @@ config_by_url(svn_config_t **cfg,
   apr_int64_t current;
 
   /* hash lookup url -> sha1 -> config */
-  in_repo_config_t *config = apr_hash_get(config_pool->in_repo_configs,
-                                          url, APR_HASH_KEY_STRING);
+  in_repo_config_t *config = svn_hash_gets(config_pool->in_repo_configs, url);
   *cfg = 0;
   if (config)
     config_ref = apr_hash_get(config_pool->configs,
@@ -574,7 +578,7 @@ svn_repos__config_pool_create(svn_repos_
 
   /* construct the config pool in our private ROOT_POOL to survive POOL
    * cleanup and to prevent threading issues with the allocator */
-  svn_repos__config_pool_t *result = apr_pcalloc(pool, sizeof(*result));
+  svn_repos__config_pool_t *result = apr_pcalloc(root_pool, sizeof(*result));
   SVN_ERR(svn_mutex__init(&result->mutex, TRUE, root_pool));
 
   result->root_pool = root_pool;
@@ -608,7 +612,7 @@ svn_repos__config_pool_get(svn_config_t 
       SVN_MUTEX__WITH_LOCK(config_pool->mutex,
                            config_by_url(cfg, config_pool, path, pool,
                                          scratch_pool));
-      if (cfg)
+      if (*cfg)
         return SVN_NO_ERROR;
 
       /* Read and cache the configuration.  This may fail. */