You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/11/02 00:13:38 UTC
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
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;