[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[Patch] Fix multiple reporting of the same lock in FSFS.

From: Sergey Raevskiy <sergey.raevskiy_at_visualsvn.com>
Date: Mon, 9 Feb 2015 18:21:03 +0300

Hi!

I've discovered interesting thing about the svn_fs_fs__get_locks() function.

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 (see the test in attached patch). Current
callers in the Subversion codebase (such as svn_repos_fs_get_locks2()) are not
affected by this bug, since they store collected locks in apr_hash_t before
reporting them to the caller.

A bit of history:

First implementation of locking in FSFS used 'recursive' approach to store
the lock indexes (see 'trunk/subversion/libsvn_fs_fs/structure'@r853645):
[[[
...

To answer the question, "Does path FOO have a lock associate with
it?", one need only generate the MD5 digest of FOO's
absolute-in-the-FS path (say, 3b1b011fed614a263986b5c4869604e8), lock
for a file located like so:

   /path/to/repos/locks/3b1/3b1b011fed614a263986b5c4869604e8

And then see if that file contains lock information.

To inquire about locks on children of the path FOO, you would
reference the same path as above, but look for a list of children in
that file (instead of lock information). Children as listed as MD5
digests, too, so you would simply iterate over those digests and
consult the files they reference, and so on, recursively.

...
]]]

Since r852750 (r12682) [1] this approach was changed to 'flat' indexes
storage (see 'trunk/subversion/libsvn_fs_fs/structure'@r956921):
[[[
 Also stored in the digest file for a given FS path are pointers to
 other digest files which contain information associated with other FS
-paths that are our path's immediate children.
+paths that are beneath our path (an immediate child thereof, or a
+grandchild, or a great-grandchild, ...).

 ...

 reference the same path as above, but look for a list of children in
 that file (instead of lock information). Children are listed as MD5
 digests, too, so you would simply iterate over those digests and
-consult the files they reference, and so on, recursively.
+consult the files they reference for lock information.
]]]

This change happened *before* locking feature has been released in Subversion
1.2. So, currently all existing FSFS repositories use 'flat' indexes for lock
storage.

However, the walk_digest_files() function wasn't updated to reflect the changes
from [1] and still iterates the locks in a 'recursive' way. I suggest removing
recursion from this function, since it can produce unexpected results for the
caller. I've attached a patch with the test and the fix for this issue.

Log message:
[[[
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
]]]

[1] http://svn.apache.org/r852750

Received on 2015-02-09 16:22:14 CET

This is an archived mail posted to the Subversion Dev mailing list.