subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1538083 - in /subversion/trunk/subversion: include/private/svn_subr_private.h libsvn_repos/config_pool.c libsvn_subr/config_file.c tests/libsvn_repos/repos-test.c
Date Fri, 01 Nov 2013 23:13:38 GMT
Author: stefan2
Date: Fri Nov  1 23:13:37 2013
New Revision: 1538083

URL: http://svn.apache.org/r1538083
Log:
Our svn_config_t struct contains tempoary string buffers that will be
modified even in r/o access mode.  Thus, it is inherently no thead-safe.

Introduce a private API function that will create shallow copies,
i.e. share the expensive config tree but use separate temp buffers.

* subversion/include/private/svn_subr_private.h
  (svn_config__shallow_copy): declare new private API function

* subversion/libsvn_subr/config_file.c
  (svn_config__shallow_copy): implement

* subversion/libsvn_repos/config_pool.c
  (return_config_ref): return shallow copies to ensure the config
                       can be used safely from multiple threads

* subversion/tests/libsvn_repos/repos-test.c
  (test_config_pool): since the config pool always returns a new root
                      struct we need to check whether the actual
                      config tree is the same

Modified:
    subversion/trunk/subversion/include/private/svn_subr_private.h
    subversion/trunk/subversion/libsvn_repos/config_pool.c
    subversion/trunk/subversion/libsvn_subr/config_file.c
    subversion/trunk/subversion/tests/libsvn_repos/repos-test.c

Modified: subversion/trunk/subversion/include/private/svn_subr_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_subr_private.h?rev=1538083&r1=1538082&r2=1538083&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_subr_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_subr_private.h Fri Nov  1 23:13:37 2013
@@ -485,6 +485,16 @@ svn_config__is_expanded(svn_config_t *cf
                         const char *section,
                         const char *option);
 
+/* Return a shallow copy of SCR in POOL.  If SRC is read-only, different
+ * shallow copies may be used from different threads.
+ *
+ * Any single r/o svn_config_t or shallow copy is not thread-safe because
+ * it contains shared buffers for tempoary data.
+ */
+svn_config_t *
+svn_config__shallow_copy(svn_config_t *src,
+                         apr_pool_t *pool);
+
 /** @} */
 
 #ifdef __cplusplus

Modified: subversion/trunk/subversion/libsvn_repos/config_pool.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/config_pool.c?rev=1538083&r1=1538082&r2=1538083&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/config_pool.c (original)
+++ subversion/trunk/subversion/libsvn_repos/config_pool.c Fri Nov  1 23:13:37 2013
@@ -227,7 +227,7 @@ return_config_ref(config_ref_t *config,
     svn_atomic_inc(&config->config_pool->used_config_count);
   apr_pool_cleanup_register(pool, config, config_ref_cleanup,
                             apr_pool_cleanup_null);
-  return config->cfg;
+  return svn_config__shallow_copy(config->cfg, pool);
 }
 
 /* Set *CFG to the configuration with a parsed textual matching CHECKSUM.

Modified: subversion/trunk/subversion/libsvn_subr/config_file.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/config_file.c?rev=1538083&r1=1538082&r2=1538083&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/config_file.c (original)
+++ subversion/trunk/subversion/libsvn_subr/config_file.c Fri Nov  1 23:13:37 2013
@@ -469,6 +469,26 @@ svn_config__is_read_only(svn_config_t *c
   return cfg->read_only;
 }
 
+svn_config_t *
+svn_config__shallow_copy(svn_config_t *src,
+                         apr_pool_t *pool)
+{
+  svn_config_t *cfg = apr_palloc(pool, sizeof(*cfg));
+
+  cfg->sections = src->sections;
+  cfg->pool = pool;
+
+  /* r/o configs are fully expanded and don't need the x_pool anymore */
+  cfg->x_pool = src->read_only ? NULL : svn_pool_create(pool);
+  cfg->x_values = src->x_values;
+  cfg->tmp_key = svn_stringbuf_create_empty(pool);
+  cfg->tmp_value = svn_stringbuf_create_empty(pool);
+  cfg->section_names_case_sensitive = src->section_names_case_sensitive;
+  cfg->option_names_case_sensitive = src->option_names_case_sensitive;
+  cfg->read_only = src->read_only;
+
+  return cfg;
+}
 
 
 svn_error_t *

Modified: subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c?rev=1538083&r1=1538082&r2=1538083&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_repos/repos-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_repos/repos-test.c Fri Nov  1 23:13:37 2013
@@ -39,6 +39,9 @@
 #include "svn_version.h"
 #include "private/svn_repos_private.h"
 
+/* be able to look into svn_config_t */
+#include "../../libsvn_subr/config_impl.h"
+
 #include "../svn_test_fs.h"
 
 #include "dir-delta-editor.h"
@@ -3311,7 +3314,8 @@ test_config_pool(const svn_test_opts_t *
   const char *repo_name = "test-repo-config-pool";
   svn_repos_t *repos;
   svn_stringbuf_t *cfg_buffer1, *cfg_buffer2;
-  svn_config_t *cfg, *cfg1, *cfg2;
+  svn_config_t *cfg;
+  apr_hash_t *sections1, *sections2;
   int i;
   svn_boolean_t bvalue;
   svn_fs_txn_t *txn;
@@ -3367,7 +3371,7 @@ test_config_pool(const svn_test_opts_t *
 
   /* requesting a config over and over again should return the same
      (even though it is not being referenced) */
-  cfg1 = NULL;
+  sections1 = NULL;
   for (i = 0; i < 4; ++i)
     {
       SVN_ERR(svn_repos__config_pool_get(
@@ -3377,10 +3381,10 @@ test_config_pool(const svn_test_opts_t *
                                                     pool),
                                     subpool));
 
-      if (cfg1 == NULL)
-        cfg1 = cfg;
+      if (sections1 == NULL)
+        sections1 = cfg->sections;
       else
-        SVN_TEST_ASSERT(cfg == cfg1);
+        SVN_TEST_ASSERT(cfg->sections == sections1);
 
       svn_pool_clear(subpool);
     }
@@ -3396,13 +3400,13 @@ test_config_pool(const svn_test_opts_t *
                                                     pool),
                                     subpool));
 
-      SVN_TEST_ASSERT(cfg == cfg1);
+      SVN_TEST_ASSERT(cfg->sections == sections1);
 
       svn_pool_clear(subpool);
     }
 
   /* reading a different configuration should return a different pointer */
-  cfg2 = NULL;
+  sections2 = NULL;
   for (i = 0; i < 2; ++i)
     {
       SVN_ERR(svn_repos__config_pool_get(
@@ -3412,12 +3416,12 @@ test_config_pool(const svn_test_opts_t *
                                                     pool),
                                     subpool));
 
-      if (cfg2 == NULL)
-        cfg2 = cfg;
+      if (sections2 == NULL)
+        sections2 = cfg->sections;
       else
-        SVN_TEST_ASSERT(cfg == cfg2);
+        SVN_TEST_ASSERT(cfg->sections == sections2);
 
-      SVN_TEST_ASSERT(cfg1 != cfg2);
+      SVN_TEST_ASSERT(sections1 != sections2);
       svn_pool_clear(subpool);
     }
 
@@ -3441,7 +3445,7 @@ test_config_pool(const svn_test_opts_t *
                                                     repo_root_url,
                                                     "dir/config", pool),
                                      subpool));
-  SVN_TEST_ASSERT(cfg == cfg1);
+  SVN_TEST_ASSERT(cfg->sections == sections1);
   svn_pool_clear(subpool);
 
   /* create another in-repo config */
@@ -3459,7 +3463,7 @@ test_config_pool(const svn_test_opts_t *
                                                     repo_root_url,
                                                     "dir/config", pool),
                                      subpool));
-  SVN_TEST_ASSERT(cfg == cfg2);
+  SVN_TEST_ASSERT(cfg->sections == sections2);
   svn_pool_clear(subpool);
 
   /* reading the copied config should still give cfg1 */
@@ -3469,7 +3473,7 @@ test_config_pool(const svn_test_opts_t *
                                                     "another-dir/config",
                                                     pool),
                                      subpool));
-  SVN_TEST_ASSERT(cfg == cfg1);
+  SVN_TEST_ASSERT(cfg->sections == sections1);
   svn_pool_clear(subpool);
 
   /* once again: repeated reads.  This triggers a different code path. */
@@ -3478,14 +3482,14 @@ test_config_pool(const svn_test_opts_t *
                                                     repo_root_url,
                                                     "dir/config", pool),
                                      subpool));
-  SVN_TEST_ASSERT(cfg == cfg2);
+  SVN_TEST_ASSERT(cfg->sections == sections2);
   SVN_ERR(svn_repos__config_pool_get(&cfg, config_pool,
                                      svn_path_url_add_component2(
                                                     repo_root_url,
                                                     "another-dir/config",
                                                     pool),
                                      subpool));
-  SVN_TEST_ASSERT(cfg == cfg1);
+  SVN_TEST_ASSERT(cfg->sections == sections1);
   svn_pool_clear(subpool);
 
   /* access paths that don't exist */
@@ -3501,7 +3505,7 @@ test_config_pool(const svn_test_opts_t *
 
   /* as long as we keep a reference to a config, clearing the config pool
      should not invalidate that reference */
-  SVN_ERR(svn_repos__config_pool_get(&cfg1, config_pool,
+  SVN_ERR(svn_repos__config_pool_get(&cfg, config_pool,
                                      svn_dirent_join(wrk_dir,
                                                      "config-pool-test1.cfg",
                                                      pool),
@@ -3510,7 +3514,7 @@ test_config_pool(const svn_test_opts_t *
   for (i = 0; i < 64000; ++i)
     apr_pcalloc(config_pool_pool, 80);
 
-  SVN_ERR(svn_config_get_bool(cfg1, &bvalue, "booleans", "true3", FALSE));
+  SVN_ERR(svn_config_get_bool(cfg, &bvalue, "booleans", "true3", FALSE));
   SVN_TEST_ASSERT(bvalue);
 
   return SVN_NO_ERROR;



Mime
View raw message