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 2012/03/20 19:31:50 UTC
svn commit: r1303070 - in /subversion/branches/1.6.x: ./
subversion/libsvn_fs_fs/ subversion/tests/cmdline/
subversion/tests/cmdline/svntest/
Author: stsp
Date: Tue Mar 20 18:31:49 2012
New Revision: 1303070
URL: http://svn.apache.org/viewvc?rev=1303070&view=rev
Log:
Reintegrate the 1.6.x-issue4129 branch.
* r1302399, r1302539, r1302591, r1302613, r1178280, r1178282, r1186121,
r1186231, r1294470
Fix issue 4129, wrong predecessor count in FSFS revision files
Justification:
Repository corruption.
Notes:
r1302399 is a comment change.
r1302539 and r1302591 add a test.
r1302613 is the fix.
r1178280, r1178282, r1186121, r1186231, and r1294470 implement
a sanity check at commit time that the test relies on.
Branch:
^/subversion/branches/1.6.x-issue4129
Votes:
+1: stsp, danielsh, philip
Modified:
subversion/branches/1.6.x/ (props changed)
subversion/branches/1.6.x/STATUS
subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.c
subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.h
subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c
subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c
subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.h
subversion/branches/1.6.x/subversion/tests/cmdline/svnadmin_tests.py
subversion/branches/1.6.x/subversion/tests/cmdline/svntest/main.py (contents, props changed)
Propchange: subversion/branches/1.6.x/
------------------------------------------------------------------------------
Merged /subversion/branches/1.6.x-issue4129:r1302627-1303068
Merged /subversion/trunk:r1178280,1178282,1186121,1186231,1294470,1302399,1302539,1302591,1302613
Modified: subversion/branches/1.6.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/STATUS (original)
+++ subversion/branches/1.6.x/STATUS Tue Mar 20 18:31:49 2012
@@ -60,19 +60,3 @@ Veto-blocked changes:
Approved changes:
=================
-
- * r1302399, r1302539, r1302591, r1302613, r1178280, r1178282, r1186121,
- r1186231, r1294470
- Fix issue 4129, wrong predecessor count in FSFS revision files
- Justification:
- Repository corruption.
- Notes:
- r1302399 is a comment change.
- r1302539 and r1302591 add a test.
- r1302613 is the fix.
- r1178280, r1178282, r1186121, r1186231, and r1294470 implement
- a sanity check at commit time that the test relies on.
- Branch:
- ^/subversion/branches/1.6.x-issue4129
- Votes:
- +1: stsp, danielsh, philip
Modified: subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.c?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.c Tue Mar 20 18:31:49 2012
@@ -1351,3 +1351,27 @@ svn_fs_fs__dag_get_copyfrom_path(const c
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_fs_fs__dag_update_ancestry(dag_node_t *target,
+ dag_node_t *source,
+ apr_pool_t *pool)
+{
+ node_revision_t *source_noderev, *target_noderev;
+
+ if (! svn_fs_fs__dag_check_mutable(target))
+ return svn_error_createf
+ (SVN_ERR_FS_NOT_MUTABLE, NULL,
+ _("Attempted to update ancestry of non-mutable node"));
+
+ SVN_ERR(get_node_revision(&source_noderev, source, pool));
+ SVN_ERR(get_node_revision(&target_noderev, target, pool));
+
+ target_noderev->predecessor_id = source->id;
+ target_noderev->predecessor_count = source_noderev->predecessor_count;
+ if (target_noderev->predecessor_count != -1)
+ target_noderev->predecessor_count++;
+
+ return svn_fs_fs__put_node_revision(target->fs, target->id, target_noderev,
+ FALSE, pool);
+}
Modified: subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.h?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.h (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_fs/dag.h Tue Mar 20 18:31:49 2012
@@ -500,6 +500,12 @@ svn_error_t *svn_fs_fs__dag_get_copyfrom
dag_node_t *node,
apr_pool_t *pool);
+/* Update *TARGET so that SOURCE is it's predecessor.
+ */
+svn_error_t *
+svn_fs_fs__dag_update_ancestry(dag_node_t *target,
+ dag_node_t *source,
+ apr_pool_t *pool);
#ifdef __cplusplus
}
#endif /* __cplusplus */
Modified: subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c Tue Mar 20 18:31:49 2012
@@ -5345,6 +5345,65 @@ write_hash_rep(svn_filesize_t *size,
return svn_stream_printf(whb->stream, pool, "ENDREP\n");
}
+/* Sanity check ROOT_NODEREV, a candidate for being the root node-revision
+ of (not yet committed) revision REV in FS. Use POOL for temporary
+ allocations.
+ */
+static svn_error_t *
+validate_root_noderev(svn_fs_t *fs,
+ node_revision_t *root_noderev,
+ svn_revnum_t rev,
+ apr_pool_t *pool)
+{
+ svn_revnum_t head_revnum = rev-1;
+ int head_predecessor_count;
+
+ SVN_ERR_ASSERT(rev > 0);
+
+ /* Compute HEAD_PREDECESSOR_COUNT. */
+ {
+ svn_fs_root_t *head_revision;
+ const svn_fs_id_t *head_root_id;
+ node_revision_t *head_root_noderev;
+
+ /* Get /@HEAD's noderev. */
+ SVN_ERR(svn_fs_fs__revision_root(&head_revision, fs, head_revnum, pool));
+ SVN_ERR(svn_fs_fs__node_id(&head_root_id, head_revision, "/", pool));
+ SVN_ERR(svn_fs_fs__get_node_revision(&head_root_noderev, fs, head_root_id,
+ pool));
+
+ head_predecessor_count = head_root_noderev->predecessor_count;
+ }
+
+ /* Check that the root noderev's predecessor count equals REV.
+
+ This kind of corruption was seen on svn.apache.org (both on
+ the root noderev and on other fspaths' noderevs); see
+ http://mid.gmane.org/20111002202833.GA12373@daniel3.local
+
+ Normally (rev == root_noderev->predecessor_count), but here we
+ use a more roundabout check that should only trigger on new instances
+ of the corruption, rather then trigger on each and every new commit
+ to a repository that has triggered the bug somewhere in its root
+ noderev's history.
+ */
+ if (root_noderev->predecessor_count != -1
+ && (root_noderev->predecessor_count - head_predecessor_count)
+ != (rev - head_revnum))
+ {
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+ _("predecessor count for "
+ "the root node-revision is wrong: "
+ "found (%d+%ld != %d), committing r%ld"),
+ head_predecessor_count,
+ rev - head_revnum, /* This is equal to 1. */
+ root_noderev->predecessor_count,
+ rev);
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* Copy a node-revision specified by id ID in fileystem FS from a
transaction into the proto-rev-file FILE. Return the offset of
the new node-revision in *OFFSET. If this is a directory, all
@@ -5360,7 +5419,11 @@ write_hash_rep(svn_filesize_t *size,
If REPS_TO_CACHE is not NULL, append to it a copy (allocated in
REPS_POOL) of each data rep that is new in this revision.
- Temporary allocations are from POOL. */
+ AT_ROOT is true if the node revision being written is the root
+ node-revision. It is only controls additional sanity checking
+ logic.
+
+ Temporary allocations are also from POOL. */
static svn_error_t *
write_final_rev(const svn_fs_id_t **new_id_p,
apr_file_t *file,
@@ -5372,6 +5435,7 @@ write_final_rev(const svn_fs_id_t **new_
apr_off_t initial_offset,
apr_array_header_t *reps_to_cache,
apr_pool_t *reps_pool,
+ svn_boolean_t at_root,
apr_pool_t *pool)
{
node_revision_t *noderev;
@@ -5410,7 +5474,7 @@ write_final_rev(const svn_fs_id_t **new_
dirent = val;
SVN_ERR(write_final_rev(&new_id, file, rev, fs, dirent->id,
start_node_id, start_copy_id, initial_offset,
- reps_to_cache, reps_pool,
+ reps_to_cache, reps_pool, FALSE,
subpool));
if (new_id && (svn_fs_fs__id_rev(new_id) == rev))
dirent->id = svn_fs_fs__id_copy(new_id, pool);
@@ -5508,6 +5572,8 @@ write_final_rev(const svn_fs_id_t **new_
noderev->id = new_id;
/* Write out our new node-revision. */
+ if (at_root)
+ SVN_ERR(validate_root_noderev(fs, noderev, rev, pool));
SVN_ERR(svn_fs_fs__write_noderev(svn_stream_from_aprfile2(file, TRUE, pool),
noderev, ffd->format,
svn_fs_fs__fs_supports_mergeinfo(fs),
@@ -5818,7 +5884,7 @@ commit_body(void *baton, apr_pool_t *poo
root_id = svn_fs_fs__id_txn_create("0", "0", cb->txn->id, pool);
SVN_ERR(write_final_rev(&new_root_id, proto_file, new_rev, cb->fs, root_id,
start_node_id, start_copy_id, initial_offset,
- cb->reps_to_cache, cb->reps_pool,
+ cb->reps_to_cache, cb->reps_pool, TRUE,
pool));
/* Write the changed-path information. */
Modified: subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c Tue Mar 20 18:31:49 2012
@@ -866,11 +866,11 @@ add_change(svn_fs_t *fs,
/* Get the id of a node referenced by path PATH in ROOT. Return the
id in *ID_P allocated in POOL. */
-static svn_error_t *
-fs_node_id(const svn_fs_id_t **id_p,
- svn_fs_root_t *root,
- const char *path,
- apr_pool_t *pool)
+svn_error_t *
+svn_fs_fs__node_id(const svn_fs_id_t **id_p,
+ svn_fs_root_t *root,
+ const char *path,
+ apr_pool_t *pool)
{
if ((! root->is_txn_root)
&& (path[0] == '\0' || ((path[0] == '/') && (path[1] == '\0'))))
@@ -935,7 +935,7 @@ node_kind(svn_node_kind_t *kind_p,
dag_node_t *node;
/* Get the node id. */
- SVN_ERR(fs_node_id(&node_id, root, path, pool));
+ SVN_ERR(svn_fs_fs__node_id(&node_id, root, path, pool));
/* Use the node id to get the real kind. */
SVN_ERR(svn_fs_fs__dag_get_node(&node, root->fs, node_id, pool));
@@ -1138,30 +1138,6 @@ get_root(dag_node_t **node, svn_fs_root_
}
-static svn_error_t *
-update_ancestry(svn_fs_t *fs,
- const svn_fs_id_t *source_id,
- const svn_fs_id_t *target_id,
- const char *target_path,
- int source_pred_count,
- apr_pool_t *pool)
-{
- node_revision_t *noderev;
-
- if (svn_fs_fs__id_txn_id(target_id) == NULL)
- return svn_error_createf
- (SVN_ERR_FS_NOT_MUTABLE, NULL,
- _("Unexpected immutable node at '%s'"), target_path);
-
- SVN_ERR(svn_fs_fs__get_node_revision(&noderev, fs, target_id, pool));
- noderev->predecessor_id = source_id;
- noderev->predecessor_count = source_pred_count;
- if (noderev->predecessor_count != -1)
- noderev->predecessor_count++;
- return svn_fs_fs__put_node_revision(fs, target_id, noderev, FALSE, pool);
-}
-
-
/* Set the contents of CONFLICT_PATH to PATH, and return an
SVN_ERR_FS_CONFLICT error that indicates that there was a conflict
at PATH. Perform all allocations in POOL (except the allocation of
@@ -1213,7 +1189,6 @@ merge(svn_stringbuf_t *conflict_p,
apr_hash_index_t *hi;
svn_fs_t *fs;
apr_pool_t *iterpool;
- int pred_count;
apr_int64_t mergeinfo_increment = 0;
/* Make sure everyone comes from the same filesystem. */
@@ -1541,9 +1516,7 @@ merge(svn_stringbuf_t *conflict_p,
}
svn_pool_destroy(iterpool);
- SVN_ERR(svn_fs_fs__dag_get_predecessor_count(&pred_count, source, pool));
- SVN_ERR(update_ancestry(fs, source_id, target_id, target_path,
- pred_count, pool));
+ SVN_ERR(svn_fs_fs__dag_update_ancestry(target, source, pool));
if (svn_fs_fs__fs_supports_mergeinfo(fs))
SVN_ERR(svn_fs_fs__dag_increment_mergeinfo_count(target,
@@ -2983,7 +2956,7 @@ fs_node_origin_rev(svn_revnum_t *revisio
path = svn_fs__canonicalize_abspath(path, pool);
/* Check the cache first. */
- SVN_ERR(fs_node_id(&given_noderev_id, root, path, pool));
+ SVN_ERR(svn_fs_fs__node_id(&given_noderev_id, root, path, pool));
node_id = svn_fs_fs__id_node_id(given_noderev_id);
/* Is it a brand new uncommitted node? */
@@ -3055,7 +3028,7 @@ fs_node_origin_rev(svn_revnum_t *revisio
}
/* Walk the predecessor links back to origin. */
- SVN_ERR(fs_node_id(&pred_id, curroot, lastpath->data, predidpool));
+ SVN_ERR(svn_fs_fs__node_id(&pred_id, curroot, lastpath->data, predidpool));
while (pred_id)
{
svn_pool_clear(subpool);
@@ -3681,7 +3654,7 @@ static root_vtable_t root_vtable = {
fs_paths_changed,
svn_fs_fs__check_path,
fs_node_history,
- fs_node_id,
+ svn_fs_fs__node_id,
svn_fs_fs__node_created_rev,
fs_node_origin_rev,
fs_node_created_path,
Modified: subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.h?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.h (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.h Tue Mar 20 18:31:49 2012
@@ -55,6 +55,13 @@ svn_fs_fs__check_path(svn_node_kind_t *k
const char *path,
apr_pool_t *pool);
+/* Implement root_vtable_t.node_id(). */
+svn_error_t *
+svn_fs_fs__node_id(const svn_fs_id_t **id_p,
+ svn_fs_root_t *root,
+ const char *path,
+ apr_pool_t *pool);
+
/* Set *REVISION to the revision in which PATH under ROOT was created.
Use POOL for any temporary allocations. If PATH is in an
uncommitted transaction, *REVISION will be set to
Modified: subversion/branches/1.6.x/subversion/tests/cmdline/svnadmin_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/tests/cmdline/svnadmin_tests.py?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/tests/cmdline/svnadmin_tests.py (original)
+++ subversion/branches/1.6.x/subversion/tests/cmdline/svnadmin_tests.py Tue Mar 20 18:31:49 2012
@@ -20,6 +20,7 @@
import os
import shutil
import sys
+import threading
# Our testing module
import svntest
@@ -951,6 +952,45 @@ def verify_with_invalid_revprops(sbox):
raise svntest.Failure
+def mergeinfo_race(sbox):
+ "concurrent mergeinfo commits invalidate pred-count"
+ sbox.build()
+
+ wc_dir = sbox.wc_dir
+ wc2_dir = sbox.add_wc_path('2')
+
+ ## Create wc2.
+ svntest.main.run_svn(None, 'checkout', '-q', sbox.repo_url, wc2_dir)
+
+ ## Some random edits.
+ svntest.main.run_svn(None, 'mkdir', os.path.join(wc_dir, 'd1'))
+ svntest.main.run_svn(None, 'mkdir', os.path.join(wc2_dir, 'd2'))
+
+ ## Set random mergeinfo properties.
+ svntest.main.run_svn(None, 'ps', 'svn:mergeinfo', '/P:42', os.path.join(wc_dir, 'A'))
+ svntest.main.run_svn(None, 'ps', 'svn:mergeinfo', '/Q:42', os.path.join(wc2_dir, 'iota'))
+
+ def makethread(some_wc_dir):
+ def worker():
+ svntest.main.run_svn(None, 'commit', '-mm', some_wc_dir)
+ return worker
+
+ t1 = threading.Thread(None, makethread(wc_dir))
+ t2 = threading.Thread(None, makethread(wc2_dir))
+
+ # t2 will trigger the issue #4129 sanity check in fs_fs.c
+ t1.start(); t2.start();
+
+ t1.join(); t2.join();
+
+ # Crude attempt to make sure everything worked.
+ # TODO: better way to catch exceptions in the thread
+ if svntest.actions.run_and_parse_info(sbox.repo_url)[0]['Revision'] != '3':
+ raise svntest.Failure("one or both commits failed")
+
+
+
+
########################################################################
# Run the tests
@@ -978,6 +1018,8 @@ test_list = [ None,
create_in_repo_subdir,
SkipUnless(verify_with_invalid_revprops,
svntest.main.is_fs_type_fsfs),
+ SkipUnless(mergeinfo_race,
+ svntest.main.is_threaded_python),
]
if __name__ == '__main__':
Modified: subversion/branches/1.6.x/subversion/tests/cmdline/svntest/main.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/tests/cmdline/svntest/main.py?rev=1303070&r1=1303069&r2=1303070&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/tests/cmdline/svntest/main.py (original)
+++ subversion/branches/1.6.x/subversion/tests/cmdline/svntest/main.py Tue Mar 20 18:31:49 2012
@@ -980,6 +980,9 @@ def is_posix_os():
def is_os_darwin():
return sys.platform == 'darwin'
+def is_threaded_python():
+ return True
+
def server_has_mergeinfo():
_check_command_line_parsed()
return server_minor_version >= 5
Propchange: subversion/branches/1.6.x/subversion/tests/cmdline/svntest/main.py
------------------------------------------------------------------------------
Merged /subversion/trunk/subversion/tests/cmdline/svntest/main.py:r1302539,1302591,1302613
Merged /subversion/branches/1.6.x-issue4129/subversion/tests/cmdline/svntest/main.py:r1302627-1303068