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

Re: Nasty double-replace copy-on-update bug

From: David Glasser <glasser_at_davidglasser.net>
Date: 2007-10-19 14:15:45 CEST

On 10/19/07, Philip Martin <philip@codematters.co.uk> wrote:
> "Ben Collins-Sussman" <sussman@red-bean.com> writes:
>
> > The question is: why the wc-searching routine
> > (update_editor.c:locate_copyfrom()) think that it's found the correct
> > version of the file, when it's not? Answer: it seems to be getting
> > stale svn_wc_entry_t information. It's reading a couple of entries
> > files from disk, rather than from a more recent adm_access set still
> > held in memory, and getting an incorrect (stale) last-changed-rev
> > value. From poking around in gdb, it seems that the entry_t's which
> > are write-cached in memory are correct -- they have the proper
> > last-changed-rev values, but simply aren't flushed to disk yet.
> >
> > I attempted to solve this by passing in the edit_baton->access_set to
> > the searching routine, but it's not fixing the problem. I've attached
> > my patch-in-progress below. Can we get some more eyes on this
> > problem?
>
> I think there are two bugs here. I believe it is wrong for
> locate_copyfrom to use a read-only snapshot of the working copy, your
> patch to pass an access baton looks like the right thing to do,
> although I think that the _try calls should take write locks. However
> that's not what is causing the current failure.

Why is that? It sounds like a read-only operation...

> The reason that locate_copyfrom thinks it has the right file is that
> when alpha first gets added it has committed-rev=1 and that meets the
> conditions required by locate_copyfrom. This happens because alpha
> itself has copyfrom info and update_editor.c:add_file_with_history
> calls eb->fetch_func to get gamma@1 from the repository and then there
> is a bit of code that does:
>
> /* Loop over whatever base-props we have in memory, faking change_file_prop()
> calls against the file baton. */
> for (hi = apr_hash_first(pool, base_props); hi; hi = apr_hash_next(hi))
> {
> const void *key;
> void *val;
> const char *propname;
> svn_string_t *propval;
> apr_hash_this(hi, &key, NULL, &val);
> propname = key;
> propval = val;
> SVN_ERR(change_file_prop(tfb, propname, propval, pool));
> }
>
> which transfers the gamma@1 props to the new alpha, including
> committed-rev=1. Later on alpha will get corrected to have
> committed-rev=3 but before that something else happens to look for
> alpha@1.

Well, it's not really "before that". Based on the editor drive that's
happening, the server has already told the client that commited-rev=3;
this information is in a loggy that has not yet been executed. Should
the cached entries in the baton be updated when the loggy is written
or when it is executed?

See Issue 2977 for a more detailed explanation of what events occur.

> Perhaps properties like committed-rev should be filtered out of then
> above loop? Perhaps only regular props should be transferred?

Maybe, but that seems like sidestepping the problem, which is that the
left hand of the library knows something (that the server has set
committed-rev) that the right hand doesn't...

--dave

-- 
David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Oct 19 14:15:56 2007

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.