[darcs-users] darcs patch: Fix a memory leak in Darcs.Repository.Repair.

Ganesh Sittampalam ganesh at earth.li
Tue Dec 23 00:59:15 UTC 2008


On Tue, 23 Dec 2008, Petr Rockai wrote:

> As it were, it built a FL of patches, possibly replacing some with a 
> repaired version. However, since it also applied those patches, their 
> content was evaluated and never freed (since the list was needed at the 
> end of repair). What I have done now is, that we only remember the 
> patches that were actually changed, and then re-create the list of all 
> patches, and only then, I zip in those changed patches.
>
> Since the new, re-created list of patches stays unevaluated (well, the contents
> part of it, anyway), we avoid the massive memory use of the original code.
>
> The obvious problem with my current patch is the way replaceInFL works, 
> relying on PatchInfo equality on the repaired patches. I unfortunately 
> haven't had time to verify that this is always true, or whether we 
> really want to enforce this as a policy. Consider this patch a DRAFT 
> until we decide upon this issue. Also, Jason pointed out safety issues 
> in replaceInFL and proposed use of IsEq. I currently don't have time to 
> grok IsEq (I need to sleep!), but Ganesh might shed some light into that 
> issue and with his help, I might be able to add some safety to that 
> code.

Just to give a (very brief!) overview of the point of IsEq: the type 
witnesses stuff is designed so that the type system generally knows when 
two patch contexts are the same, because they are represented by the same 
type witness. Sometimes, this information (namely the equality of two 
contexts) has to be propagated around the darcs code to help the type 
system out, and the way we do this is with IsEq, which amounts to evidence 
that two type witnesses with different names are actually the same. The 
point about it is the declaration:

data EqCheck a b where
   IsEq :: EqCheck a a
   NotEq :: EqCheck a b

so if we have some value of type EqCheck a b in our hands, and we inspect 
it and it turns out that it is 'IsEq', then we have a proof that a = b.

There are cases where the type system just can't keep track of 
equality no matter how much we help it, and I think this is one of them.
The issue here is that the type witnesses won't match up between the list 
re-read in from the repo and the list of changed patches, for two reasons. 
Firstly, when we put the changed patches in a list, we lose track of the 
type witnesses they originally had. Secondly, re-reading from the repo 
will generate a fresh set of witnesses in any case. It's only our 
knowledge that reading the same repo twice generates the same sequence of 
contexts that tells us that the code is ok.

I guess the proposed patch just won't work with type witnesses as it 
stands, and will need whatever an appropriate "unsafe" operation to make 
it compile. I think lispy was suggesting on #darcs that you manufacture an 
IsEq with unsafeCoerce (i.e. just 'unsafeCoerce IsEq') in a utility 
function that compares the pinfos, but I might have misunderstood. I can 
try and help with the details if you can catch me on IRC.

Ganesh


More information about the darcs-users mailing list