[PATCH] atomictempfile: avoid infinite recursion in destructor

[Nicolas Dumazet]

2010/11/30 Greg Ward <greg-hg at gerg.ca>:
> # HG changeset patch
> # User Greg Ward <greg-hg at gerg.ca>
> # Date 1291085861 18000
> # Branch stable
> # Node ID 76a458f91f5baad3dd75a26d445ea51402fc32d1
> # Parent  5fb924ee44d516e2f91bb7f5cfd5c81b0629faa4
> atomictempfile: avoid infinite recursion in destructor.
>
> This is a weird one: if Python sees that the caller is not passing the
> right args to the constructor, it raises TypeError rather than enter
> the constructor.  But it still ends up calling the destructor, which
> assumes that self._fp has been set in the constructor.  But it hasn't,
> because the constructor wasn't actually executed.  So we go into a
> nasty infinite recursion between __del__() and __getattr__(), when we
> should just crash with TypeError so the programmer can see what he did
> wrong.
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -823,7 +823,9 @@
>             rename(self.temp, localpath(self.__name))
>
>     def __del__(self):
> -        if not self._fp:
> +        if '_fp' not in self.__dict__:  # constructor not executed
> +            return
> +        if not self._fp:                # constructor failed

That's quite interesting! :)

You might want to be verbose on purpose, but what about
  if not getattr(self, '_fp', None)
? It doesn't work because we're in __del__? (interesting case...!)

I was about to ask for a test, but fair enough, creating a .py file
just for this detail might be overkill.

Regards,

>             return
>         if not self._fp.closed:
>             try:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>



-- 
Nicolas Dumazet — NicDumZ