[PATCH] Use try/finally to close files on error

Matt Mackall mpm at selenic.com
Wed Nov 2 16:54:45 CDT 2011


On Wed, 2011-11-02 at 22:03 +0100, Victor Stinner wrote:
> Le 02/11/2011 21:47, Matt Mackall a écrit :
> >> Use "try: ...  finally: fp.close()" in other places to ensure that the
> >> file is
> >> also closed on error.
> >
> > Given that Python automatically closes files when garbage-collecting,
> > what do we gain by this?
> 
> It is not a good idea to relay on the garbage collector, it's not 
> reliable. In PyPy, the garbage collector can be called "later" (you 
> cannot know when exactly).

We're aware of that. But until you point to -actual- bugs that this
fixes, I consider this bloat. The whole point of exception propagation
is to allow code that can't meaningfully/usefully handle errors to
ignore it. For instance:

-            fp.write("".join(self._delaybuf))
-            fp.close()
+            try:
+                fp.write("".join(self._delaybuf))
+            finally:
+                fp.close()

There's not much point to this. If we fail the write, we are "screwed"
and doing a close() here doesn't make us any less "screwed". 

(As it happens, there's a whole 'nother layer of pointlessness here:
basically all modern operating systems have non-blocking writes by
default which means errors may not be encountered until the operating
system tries to flush data out of memory to the filesystem, at which
point it can encounter EIO, ENOSPC, etc. And since write() has already
returned, the next opportunity to report the error is at close() - I
wrote the code in the Linux kernel that does this about a decade ago. So
wrapping just write() in exception-handling code is fooling yourself.)

> > Your email client damages patches.
> 
> Oh sorry. I attached the patch as a file to this mail.

Won't do. Please read ContributingPatches for the rationale.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list