This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: File handle leak in TarFile lib
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: duplicate
Dependencies: Superseder: tarfile fails to close file handles in case of exception
View: 16477
Assigned To: lars.gustaebel Nosy List: eric.araujo, ezio.melotti, lars.gustaebel, serhiy.storchaka, shahpr
Priority: normal Keywords: easy, patch

Created on 2011-04-06 18:42 by shahpr, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue11787.diff ezio.melotti, 2011-04-15 06:26 Patch without tests against 2.7 review
Messages (7)
msg133153 - (view) Author: Pulin Shah (shahpr) Date: 2011-04-06 18:42
I ran into a problem the other day while trying to extract a slightly corrupted tar file.  I suspect this problem is really only an issue on Windows systems.  I am running Python 2.7.1 r271:86832 win32.

The following code (simplified) snipet

try:
	tar = tarfile.open(args.file)
	tar.extractall(basefolder)
	tar.close()
except tarfile.ReadError:
	shutil.rmtree(basefolder)
except IOError:
	shutil.rmtree(basefolder)

was throwing a WindowsError on the rmtree calls.

This is due to the tarfile library not closing file handles in the case of an exception in the copyfileobj function, and Windows inability to delete open files.  

I was able to patch the issue locally by modifying tarfile's makefile function as follows:

    def makefile(self, tarinfo, targetpath):
        """Make a file called targetpath.
        """
        source = self.extractfile(tarinfo)
        target = bltn_open(targetpath, "wb")
        try:
            copyfileobj(source, target)
        except:
            source.close()
            target.close()
            raise
        source.close()
        target.close()

There is probably a cleaner way of implementing it.

I'm hoping you can integrate this patch into later versions of the lib.

Thanks.
msg133318 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-04-08 15:47
A try/except/finally construct would probably work.
msg133795 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-04-15 06:26
Attached trivial patch to fix the issue. Needs tests.
msg133820 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-04-15 13:36
LGTM.  Is an automated test really needed, or just a manual run with a pydebug build?
msg133822 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-04-15 13:46
An automated test would be better.  It should be enough to create an invalid tar file, do something similar to the snippet in the first message, but checking that an error is raised and that all the files are closed anyway.
msg183447 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-03-04 13:21
Actually it was fixed in issue16477. ;)
msg183483 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-04 18:14
Closing as duplicate then.
History
Date User Action Args
2022-04-11 14:57:15adminsetgithub: 55996
2013-03-04 18:14:40ezio.melottisetstatus: open -> closed
superseder: tarfile fails to close file handles in case of exception
messages: + msg183483

resolution: duplicate
stage: test needed -> resolved
2013-03-04 13:21:18serhiy.storchakasetmessages: + msg183447
2013-03-02 13:20:30ezio.melottisetnosy: + serhiy.storchaka
2011-04-15 13:46:27ezio.melottisetmessages: + msg133822
2011-04-15 13:36:06eric.araujosetmessages: + msg133820
2011-04-15 06:26:05ezio.melottisetfiles: + issue11787.diff

nosy: + ezio.melotti
messages: + msg133795

keywords: + easy, patch
stage: test needed
2011-04-09 07:08:24lars.gustaebelsetassignee: lars.gustaebel
2011-04-08 15:47:10eric.araujosetnosy: + eric.araujo
messages: + msg133318
2011-04-06 21:19:27ned.deilysetnosy: + lars.gustaebel
2011-04-06 18:42:54shahprcreate