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

Re: log cache bugs

From: Stefan Fuhrmann <stefanfuhrmann_at_alice-dsl.de>
Date: Sat, 28 Mar 2009 21:35:40 +0100

Stefan Küng wrote:

> I guess these are for you:
>
> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=
>1411719
>
> 1. build a new folder c:\^svn
> 2. right click it, select "TortoiseSVN"->"Create repository here"
> 3. build a new folder c:\WorkCopy
> 4. right click it, select "SVN Checkout..."
> 5. use "file:///c:/^svn" as the url, do checkout
> 6. right click the workcopy folder, select "TortoiseSVN"->"Revision graph"
> You will get a crash-report window instead of the graph.
>
> Problem here is that in
> CCachedLogInfo* CLogCachePool::GetCache (const CString& uuid, const
> CString& root)
>
> the call to
> CRepositoryInfo::SPerRepositoryInfo* info
> = repositoryInfo->data.Lookup (uuid, root);
>
> returns NULL for info (if GetAllowAmbiguousUUID and GetAllowAmbiguousURL
> both return TRUE).

Yep. The code expects a valid root URL or unique UUID.
It assumes that UUID and root have been determined before.
The assert() in the following line tests that condition.

> On a side note: I think you have inverted the if-clause there!

If you are referring to CRepositoryInfo::CData::Lookup(),
those if-clauses are correct:

* if (uuid, root) matches an existing cache, that is the one
  we want as the key pair must *always* be unique
  (the log cache does not support 2 repositories with the same
  URL and UUID).
* if the uuid must be unique (i.e. !GetAllowAmbiguousUUID() ),
  try a lookup by uuid alone. Since the uuid may be 0, continue
  if there was no match
* if the path must be unique (i.e. !GetAllowAmbiguousURL() ),
  try a uuid-agnostic lookup, i.e. by root alone.

> Please review my fix for this (r15923)

The fix is correct - it actually was a revision graph problem:
the log cache was called with invalid parameters.

r15967 polishes the code a bit but doesn't need to be backported.

> Another issue is reported here:
> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=
>1416518
>
> I've seen the same issue with the TSVN working copy: showing the log
> from the wc root (trunk) worked ok, but the log for /trunk/src or
> /trunk/src/TortoiseProc would loop endlessly. The same of course happens
> when trying to show the blame for a file inside those subfolders.
>
> Running the log in the debugger shows that the assertion in line 1007 of
> the file CacheLogQuery.cpp is hit. And as the comment there tells: it
> causes an indefinite loop.
>
> Unfortunately, I cleared the log cache after that to see if that makes a
> difference, and it does. But now I don't have the cache anymore :(

Well, clearing the cache actually made things "worse".

> I've added a fix in r15928 for this, but I guess you'd rather knew why
> that situation occurred in the first place...

The cause for that behavior turned out to be in a different part
of the cache than similar bugs we had in previous releases.
So, with r15971, we should finally have found the last one
(can't believe I'm giving such stupid statements ;) ).

For the records, that is what happened:

* empty log cache or one with plenty of gaps in it
* show log for some close-to-root path for a repository with
  - at least some 100 revisions
  - at least one of the last 100 revisions was not on that path
  - the result not covering all history for that path
* results in a proper log, cache content and a non-empty
  skip-range list for this path. The latter is important.

* show log for some sub-path of the former path with
  - last 100 changing revisions span a longer revision
    range than the one of the parent paths.
  - no skip range info on the former path for the oldest
    revisions in the sub-path history (i.e. revisions that
    have not been seen before for any parent path)

* FillLog will update the log cache with info that it just
  received for revisions that had previously been in cache
  (i.e. those that had already been shown for the parent path).
* This will cause the skip range info to be compressed,
  i.e. redundant info gets removed.

* Bug: all entries for revisions older than the oldest
  entry for a parent path were removed (but only if there
  was skip-range info any parent at all).
* Result: cannot iterate over older revisions as the gaps
  cannot be skipped. The next FillLog will trigger compression
  again and no progress can be made.

BTW, that cannot happen for the revision graph, as it fetches
the log at the repository root and the result has no gaps.

-- Stefan^2.
Received on 2009-03-28 21:35:50 CET

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

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