Stefan Fuhrmann wrote on Sat, Jun 11, 2011 at 10:13:27 +0200:
> On 10.06.2011 22:28, Daniel Shahaf wrote:
> >stefan2_at_apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
> >>Author: stefan2
> >>Date: Wed May 25 22:20:25 2011
> >>New Revision: 1127709
> >>
> >>URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
> >>Log:
> >>Fix a pool usage issue: svn_cache__get_partial() may be called many
> >>times in a row. Thus, the internal pool used to construct keys should
> >>be cleared in this function as well from time to time.
> >>
> >>* subversion/libsvn_subr/cache-membuffer.c
> >> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
> >>
> >>Modified:
> >> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>
> >>Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
> >>==============================================================================
> >>--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
> >>+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
> >>@@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
> >>
> >> DEBUG_CACHE_MEMBUFFER_INIT_TAG
> >>
> >>+ if (++cache->alloc_counter> ALLOCATIONS_PER_POOL_CLEAR)
> >>+ {
> >>+ apr_pool_clear(cache->pool);
> >>+ cache->alloc_counter = 0;
> >>+ }
> >>+
> >Does this need to be guarded by a cache lock?
> >
> No. This happens in the outer / front-end code
> that merely adds a key prefix (combine_key below)
> before calling the shared cache object.
>
> All front-end operations assume single-threaded
> access, which should be o.k. for fs_t-local objects.
Okay, if that function is guaranteed (perhaps by API contract) not to
run concurrently to any other `front end' function (and in particular to
itself), then my concerns are resolved.
(I was worried about accessing cache->pool and cache->alloc_counter from
multiple threads concurrently --- writer-writer or writer-reader ---
which might to undefined behaviour.)
What guarantees the single-threaded access? I don't see it documented
in svn_cache.h (on the contrary, that one has explicit 'thread_safe'
parameters) and the code doesn't take a lock at at that point either.
> >> combine_key(cache->prefix,
> >> sizeof(cache->prefix),
> >> key,
> >
> -- Stefan^2.
Received on 2011-06-11 14:53:57 CEST