[issue16477] tarfile fails to close file handles in case of exception

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> Would "with source = self.extractfile(tarinfo):" work?

No.  extractfile() can return an instance of custom class.  But in 3.x this not 
used at all.

Here are patches for 2.7 and 3.x.

----------
keywords: +patch
Added file: http://bugs.python.org/file28012/tarfile_fd_leaks-2.7.patch
Added file: http://bugs.python.org/file28013/tarfile_fd_leaks.patch

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue16477>
_______________________________________
-------------- next part --------------
diff -r 457c0c9c7893 Lib/tarfile.py
--- a/Lib/tarfile.py	Thu Nov 15 07:10:27 2012 +0100
+++ b/Lib/tarfile.py	Sat Nov 17 21:57:57 2012 +0200
@@ -1987,9 +1987,8 @@
 
         # Append the tar header and data to the archive.
         if tarinfo.isreg():
-            f = bltn_open(name, "rb")
-            self.addfile(tarinfo, f)
-            f.close()
+            with bltn_open(name, "rb") as f:
+                self.addfile(tarinfo, f)
 
         elif tarinfo.isdir():
             self.addfile(tarinfo)
@@ -2197,10 +2196,11 @@
         """Make a file called targetpath.
         """
         source = self.extractfile(tarinfo)
-        target = bltn_open(targetpath, "wb")
-        copyfileobj(source, target)
-        source.close()
-        target.close()
+        try:
+            with bltn_open(targetpath, "wb") as target:
+                copyfileobj(source, target)
+        finally:
+            source.close()
 
     def makeunknown(self, tarinfo, targetpath):
         """Make a file from a TarInfo object with an unknown type
-------------- next part --------------
diff -r e9af9b1ca67e Lib/tarfile.py
--- a/Lib/tarfile.py	Sat Nov 17 19:18:10 2012 +0000
+++ b/Lib/tarfile.py	Sat Nov 17 21:37:45 2012 +0200
@@ -1924,9 +1924,8 @@
 
         # Append the tar header and data to the archive.
         if tarinfo.isreg():
-            f = bltn_open(name, "rb")
-            self.addfile(tarinfo, f)
-            f.close()
+            with bltn_open(name, "rb") as f:
+                self.addfile(tarinfo, f)
 
         elif tarinfo.isdir():
             self.addfile(tarinfo)
@@ -2131,16 +2130,15 @@
         """
         source = self.fileobj
         source.seek(tarinfo.offset_data)
-        target = bltn_open(targetpath, "wb")
-        if tarinfo.sparse is not None:
-            for offset, size in tarinfo.sparse:
-                target.seek(offset)
-                copyfileobj(source, target, size)
-        else:
-            copyfileobj(source, target, tarinfo.size)
-        target.seek(tarinfo.size)
-        target.truncate()
-        target.close()
+        with bltn_open(targetpath, "wb") as target:
+            if tarinfo.sparse is not None:
+                for offset, size in tarinfo.sparse:
+                    target.seek(offset)
+                    copyfileobj(source, target, size)
+            else:
+                copyfileobj(source, target, tarinfo.size)
+            target.seek(tarinfo.size)
+            target.truncate()
 
     def makeunknown(self, tarinfo, targetpath):
         """Make a file from a TarInfo object with an unknown type