Package: subversion
Version: 1.8.5-1
Severity: minor
Tags: upstream
There is also another bug Subversion 1.8 libsvn_subr - cache->hit_count
and entry->hit_count are incremented without proper mutual exclusion
(only with read lock), so some increments get lost in multithreaded
svnserve; as it's unlikely two threads access the same entry at once,
usually cache->hit_count (not entry->hit_count) increments are lost,
which leads to cache->hit_count being less than sum(entry->hit_count).
So it may overflow during subtraction of entry->hit_count and lead to
removing of values that should not be removed from the cache in
ensure_data_insertable(). Instead of killing the performance by mutual
exclusion we may just check for possible overflow.
This doesn't affect general svnserve usability, so severity=minor.
A patch is attached; this is also an upstream issue, so I'll also try to
send it to authors.
See the comment.
--- a/subversion/libsvn_subr/cache-membuffer.c 2014-02-12 21:53:58.547230675
+0000
+++ a/subversion/libsvn_subr/cache-membuffer.c 2014-02-12 22:02:11.039246322
+0000
@@ -639,9 +639,16 @@ drop_entry(svn_membuffer_t *cache, entry
assert(idx <= last_in_group);
/* update global cache usage counters
+ *
+ * cache->hit_count and entry->hit_count are incremented without proper
mutual
+ * exclusion (only with read lock), so some increments get lost in
multithreaded svnserve;
+ * as it's unlikely two threads access the same entry at once usually it
leads to
+ * cache->hit_count being less than sum(entry->hit_count). So it may overflow
+ * during subtraction and lead to emptying the cache in
ensure_data_insertable().
+ * instead of killing the performance by mutual exclusion we just check for
possible overflow.
*/
cache->used_entries--;
- cache->hit_count -= entry->hit_count;
+ cache->hit_count = cache->hit_count < entry->hit_count ? 0 :
cache->hit_count-entry->hit_count;
cache->data_used -= entry->size;
/* extend the insertion window, if the entry happens to border it
@@ -802,7 +809,7 @@ let_entry_age(svn_membuffer_t *cache, en
{
apr_uint32_t hits_removed = (entry->hit_count + 1) >> 1;
- cache->hit_count -= hits_removed;
+ cache->hit_count = cache->hit_count < hits_removed ? 0 :
cache->hit_count-hits_removed;
entry->hit_count -= hits_removed;
}
