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

[PATCH] in-process packed-offset cache broken in 1.6.16 (on 64-bit architectures)

From: Roderich Schupp <roderich.schupp_at_googlemail.com>
Date: Sun, 15 May 2011 19:50:53 +0200


while looking at strace's of Subversion operations on a repository
using packing
I noticed the following behavior:

20829 open(".../db/revs/2.pack/pack", O_RDONLY|O_CLOEXEC) = 13
20829 open(".../db/revs/2.pack/manifest", O_RDONLY|O_CLOEXEC) = 14
20829 open(".../db/revs/2.pack/pack", O_RDONLY|O_CLOEXEC) = 14
20829 open(".../db/revs/2.pack/manifest", O_RDONLY|O_CLOEXEC) = 15
20829 open(".../db/revs/2.pack/pack", O_RDONLY|O_CLOEXEC) = 12
20829 open(".../db/revs/2.pack/manifest", O_RDONLY|O_CLOEXEC) = 13

i.e. every open for a pack file is paired with an open for the corresponding
manifest file. I would have expected that a specific manifest file is only
read once and then its contents are cached in the packed_offset_cache field
of the fs_fs_data_t struct. I saw this on Linux on amd64, i.e. where

  sizeof(int) < sizeof(rev_num_t)

The packed_offset_cache never finds anything because of the following mismatch:
- in subversion/libsvn_fs_fs/caching.c packed_offset_cache is created
with a key length of
- in subversion/libsvn_fs_fs/fs_fs.c in function get_packed_offset the
shard number is passed
  to svn_cache__{get,set} as (the address of) an int

Patch below. The problem is still present in branches/1.6.x, while trunk has
rewritten packed_offset_cache() and shard is now an apr_int64_t (which
will work,
though still not literally consistent with the creation of the cache).

Cheers, Roderich

--- subversion-1.6.16.origsubversion/libsvn_fs_fs/fs_fs.c 2011-02-14
21:54:43.000000000 +0100
+++ subversion-1.6.16/subversion/libsvn_fs_fs/fs_fs.c 2011-05-14
22:11:39.127424984 +0200
@@ -1800,7 +1800,7 @@
   fs_fs_data_t *ffd = fs->fsap_data;
   svn_stream_t *manifest_stream;
   svn_boolean_t is_cached;
- int shard;
+ svn_revnum_t shard;
   apr_array_header_t *manifest;
   apr_pool_t *iterpool;
Received on 2011-05-15 19:51:21 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.