subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From phi...@apache.org
Subject svn commit: r1658482 - in /subversion/trunk/subversion: libsvn_fs_fs/lock.c tests/libsvn_fs/locks-test.c
Date Mon, 09 Feb 2015 17:44:43 GMT
Author: philip
Date: Mon Feb  9 17:44:43 2015
New Revision: 1658482

URL: http://svn.apache.org/r1658482
Log:
Fix multiple reporting of the same lock in FSFS.

In some unusual (but not impossible) scenarios this function could report the
same lock multiple times: there should be the path with lock, and one of its
children should be locked as well.

* subversion/libsvn_fs_fs/lock.c
  (walk_digests_callback_t): Remove unused argument and update comment.
  (locks_walker): Update callback.
  (walk_digest_files): Don't walk digest files recursively.

* subversion/tests/libsvn_fs/locks-test.c
  (get_locks_callback): Check if a lock was already reported.
  (parent_and_child_lock): New.
  (test_funcs): Add new test.

Patch by: sergey.raevskiy{_AT_}visualsvn.com

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/lock.c
    subversion/trunk/subversion/tests/libsvn_fs/locks-test.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/lock.c?rev=1658482&r1=1658481&r2=1658482&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/lock.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/lock.c Mon Feb  9 17:44:43 2015
@@ -539,7 +539,6 @@ static svn_error_t *
 locks_walker(void *baton,
              const char *fs_path,
              const char *digest_path,
-             apr_hash_t *children,
              svn_lock_t *lock,
              svn_boolean_t have_write_lock,
              apr_pool_t *pool)
@@ -569,17 +568,16 @@ locks_walker(void *baton,
 
 /* Callback type for walk_digest_files().
  *
- * CHILDREN and LOCK come from a read_digest_file(digest_path) call.
+ * LOCK come from a read_digest_file(digest_path) call.
  */
 typedef svn_error_t *(*walk_digests_callback_t)(void *baton,
                                                 const char *fs_path,
                                                 const char *digest_path,
-                                                apr_hash_t *children,
                                                 svn_lock_t *lock,
                                                 svn_boolean_t have_write_lock,
                                                 apr_pool_t *pool);
 
-/* A recursive function that calls WALK_DIGESTS_FUNC/WALK_DIGESTS_BATON for
+/* A function that calls WALK_DIGESTS_FUNC/WALK_DIGESTS_BATON for
    all lock digest files in and under PATH in FS.
    HAVE_WRITE_LOCK should be true if the caller (directly or indirectly)
    has the FS write lock. */
@@ -599,11 +597,10 @@ walk_digest_files(const char *fs_path,
   /* First, send up any locks in the current digest file. */
   SVN_ERR(read_digest_file(&children, &lock, fs_path, digest_path, pool));
 
-  SVN_ERR(walk_digests_func(walk_digests_baton, fs_path, digest_path,
-                            children, lock,
+  SVN_ERR(walk_digests_func(walk_digests_baton, fs_path, digest_path, lock,
                             have_write_lock, pool));
 
-  /* Now, recurse on this thing's child entries (if any; bail otherwise). */
+  /* Now, report all the child entries (if any; bail otherwise). */
   if (! apr_hash_count(children))
     return SVN_NO_ERROR;
   subpool = svn_pool_create(pool);
@@ -611,15 +608,19 @@ walk_digest_files(const char *fs_path,
     {
       const char *digest = apr_hash_this_key(hi);
       svn_pool_clear(subpool);
-      SVN_ERR(walk_digest_files
-              (fs_path, digest_path_from_digest(fs_path, digest, subpool),
-               walk_digests_func, walk_digests_baton, have_write_lock, subpool));
+
+      SVN_ERR(read_digest_file
+              (NULL, &lock, fs_path,
+               digest_path_from_digest(fs_path, digest, subpool), subpool));
+
+      SVN_ERR(walk_digests_func(walk_digests_baton, fs_path, digest_path, lock,
+                                have_write_lock, subpool));
     }
   svn_pool_destroy(subpool);
   return SVN_NO_ERROR;
 }
 
-/* A recursive function that calls GET_LOCKS_FUNC/GET_LOCKS_BATON for
+/* A function that calls GET_LOCKS_FUNC/GET_LOCKS_BATON for
    all locks in and under PATH in FS.
    HAVE_WRITE_LOCK should be true if the caller (directly or indirectly)
    has the FS write lock. */

Modified: subversion/trunk/subversion/tests/libsvn_fs/locks-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/locks-test.c?rev=1658482&r1=1658481&r2=1658482&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs/locks-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs/locks-test.c Mon Feb  9 17:44:43 2015
@@ -53,9 +53,19 @@ get_locks_callback(void *baton,
   struct get_locks_baton_t *b = baton;
   apr_pool_t *hash_pool = apr_hash_pool_get(b->locks);
   svn_string_t *lock_path = svn_string_create(lock->path, hash_pool);
-  apr_hash_set(b->locks, lock_path->data, lock_path->len,
-               svn_lock_dup(lock, hash_pool));
-  return SVN_NO_ERROR;
+
+  if (!apr_hash_get(b->locks, lock_path->data, lock_path->len))
+    {
+      apr_hash_set(b->locks, lock_path->data, lock_path->len,
+                   svn_lock_dup(lock, hash_pool));
+      return SVN_NO_ERROR; 
+    }
+  else
+    {
+      return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+                               "Lock for path '%s' is being reported twice.",
+                               lock->path);
+    }
 }
 
 /* A factory function. */
@@ -1134,6 +1144,72 @@ obtain_write_lock_failure(const svn_test
   return SVN_NO_ERROR;
 }
 
+static svn_error_t *
+parent_and_child_lock(const svn_test_opts_t *opts,
+                      apr_pool_t *pool)
+{
+  svn_fs_t *fs;
+  svn_fs_access_t *access;
+  svn_fs_txn_t *txn;
+  svn_fs_root_t *root;
+  const char *conflict;
+  svn_revnum_t newrev;
+  svn_lock_t *lock;
+  struct get_locks_baton_t *get_locks_baton;
+  apr_size_t num_expected_paths;
+
+  SVN_ERR(svn_test__create_fs(&fs, "test-parent-and-child-lock", opts, pool));
+  SVN_ERR(svn_fs_create_access(&access, "bubba", pool));
+  SVN_ERR(svn_fs_set_access(fs, access));
+
+  /* Make a file '/A'. */
+  SVN_ERR(svn_fs_begin_txn(&txn, fs, 0, pool));
+  SVN_ERR(svn_fs_txn_root(&root, txn, pool));
+  SVN_ERR(svn_fs_make_file(root, "/A", pool));
+  SVN_ERR(svn_fs_commit_txn(&conflict, &newrev, txn, pool));
+
+  /* Obtain a lock on '/A'. */
+  SVN_ERR(svn_fs_lock(&lock, fs, "/A", NULL, NULL, FALSE, 0, newrev, FALSE,
+                      pool));
+
+  /* Add a lock token to FS access context. */
+  SVN_ERR(svn_fs_access_add_lock_token(access, lock->token));
+
+  /* Make some weird change: replace file '/A' by a directory with a
+     child.  Issue 2507 means that the result is that the directory /A
+     remains locked. */
+  SVN_ERR(svn_fs_begin_txn(&txn, fs, newrev, pool));
+  SVN_ERR(svn_fs_txn_root(&root, txn, pool));
+  SVN_ERR(svn_fs_delete(root, "/A", pool));
+  SVN_ERR(svn_fs_make_dir(root, "/A", pool));
+  SVN_ERR(svn_fs_make_file(root, "/A/b", pool));
+  SVN_ERR(svn_fs_commit_txn(&conflict, &newrev, txn, pool));
+
+  /* Obtain a lock on '/A/b'.  Issue 2507 means that the lock index
+     for / refers to both /A and /A/b, and that the lock index for /A
+     refers to /A/b. */
+  SVN_ERR(svn_fs_lock(&lock, fs, "/A/b", NULL, NULL, FALSE, 0, newrev, FALSE,
+                      pool));
+
+  /* Verify the locked paths. The lock for /A/b should not be reported
+     twice even though issue 2507 means we access the index for / and
+     the index for /A both of which refer to /A/b. */
+  {
+    static const char *expected_paths[] = {
+      "/A",
+      "/A/b",
+    };
+    num_expected_paths = sizeof(expected_paths) / sizeof(const char *);
+    get_locks_baton = make_get_locks_baton(pool);
+    SVN_ERR(svn_fs_get_locks(fs, "/", get_locks_callback,
+                             get_locks_baton, pool));
+    SVN_ERR(verify_matching_lock_paths(get_locks_baton, expected_paths,
+                                       num_expected_paths, pool));
+  }
+
+  return SVN_NO_ERROR;
+}
+
 /* ------------------------------------------------------------------------ */
 
 /* The test table.  */
@@ -1171,6 +1247,8 @@ static struct svn_test_descriptor_t test
                        "lock callback error"),
     SVN_TEST_OPTS_PASS(obtain_write_lock_failure,
                        "lock/unlock when 'write-lock' couldn't be obtained"),
+    SVN_TEST_OPTS_PASS(parent_and_child_lock,
+                       "lock parent and it's child"),
     SVN_TEST_NULL
   };
 



Mime
View raw message