[PATCH STABLE] localrepo: clear filecache correctly on destroyed()

[Idan Kamara]

On Tue, Nov 20, 2012 at 12:12 AM, Hao Lian <hao at fogcreek.com> wrote:
> # HG changeset patch
> # User Hao Lian <hao at fogcreek.com>
> # Date 1353360901 18000
> # Branch stable
> # Node ID 0d053ca02a3d94e56634091690a9a58c8d833329
> # Parent  35ba170c0f82dba18f0207ef4bd93216e6de8bbf
> localrepo: clear filecache correctly on destroyed()

This is a pretty complex area so going over this at work will
be hard, I'll look at it carefully during the weekend (or before).

>
> Assume R is a localrepo and P is a property of R decorated with storecache.
> Prior to 9f94358, invalidate() enforced the invariant that P is a member of
> R.__dict__ if and only if it were also mirrored in R's filecache.  Now, however,
> P may be deleted from filecache without the corresponding object being deleted
> in R.__dict__.  This allows the following bug to occur:
>
> * P is deleted from R's filecache.
> * The underlying file that P corresponds to is updated.
> * Someone attempts to access P.
> * Because the stale P is in __dict__, storecache returns stale P.
>
> Prior to 9f94358, P would have been deleted from R's both __dict__ and
> filecache, never just one of them.
>
> This exact bug occurs during hg convert, which rolls back empty revisions when
> converting Subversion repositories.  When that happens, _rollback() calls
> destroyed(), which deletes the changelog object from filecache without deleting
> it from __dict__.  As a result, the changelog becomes stale and len(changelog)
> is one larger than the actual length of the changelog; as the conversion
> progresses, hg convert (sometimes) errors out during an integrity check on the
> changelog.

A test for the behaviour you mention above would be really good due
to the buggy past of this piece of code.