Issue 12391: packaging install fails to clean up temp files (original) (raw)

Issue12391

Created on 2011-06-23 07:43 by vinay.sajip, last changed 2022-04-11 14:57 by admin. This issue is now closed.

| Repositories containing patches | | | | | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | | | | http://hg.python.org/sandbox/vsajip#fix12391 | | | |

Messages (5)
msg138860 - (view)	Author: Vinay Sajip (vinay.sajip) *	Date: 2011-06-23 07:43
There are a number of places in packaging.install where temporary directories are created, but never cleaned up: 1. In _move_files, if no destination path is passed in, one is created using mkdtemp(), but it's not clear where this would be deleted. Moreover, it's never called without a path and not part of the public API, so it would make sense to always expect a destination to be passed in (and update the docstring to match) 2. install_local_project, in the case of an archive, unpacks it into a mkdtemp()'d directory, but never deletes that directory later. 3. install_dists() also calls mkdtemp() if a path is not passed in, but it's not clear where this would be deleted. This should be changed to always require a path to be passed in. The install_from_infos accepts None as an install path and passes that to install_dists, but why are we being so generous? It's not asking a lot to be given an explicit path to install to. Note: the DistInfo class in packaging.pypi.dist also does this kind of thing (in the download and unpack methods) - it would seem sensible to make similar changes there.
msg139771 - (view)	Author: Éric Araujo (eric.araujo) *	Date: 2011-07-04 14:11
1: Agreed. 2 and 3: Alexis most probably added that behavior as a convenience. Unless I’m mistaken, the point of TMP/TMP/TMP/TMPDIR is that the OS itself will clean it up, for example on shutdown, so programs that leave stuff here are not strictly wrong. However, given the realities of Windows behavior (I recall seeing “temporary” directories with tons of stuff never cleaned up) and the low cost of a change (“It's not asking a lot to be given an explicit path to install to” +1), my opinion is that we should take your patch as it is.
msg139777 - (view)	Author: Vinay Sajip (vinay.sajip) *	Date: 2011-07-04 14:38
> Éric Araujo <merwok@netwok.org> added the comment: > 2 and 3: Alexis most probably added that behavior as a convenience. Unless >I’m mistaken, the point of TMP/TMP/TMP/TMPDIR is that the OS itself will clean it up, >for example on shutdown, so programs that leave stuff here are not strictly >wrong. However, given the realities of Windows behavior (I recall seeing >“temporary” directories with tons of stuff never cleaned up) and the low cost >of a change (“It's not asking a lot to be given an explicit path to install to” >+1), my opinion is that we should take your patch as it is. Great. Although /tmp is cleaned up on restart (on Linux at least), waiting for that can lead to problems. For example, I came across this problem when I (for test purposes) installed every single one of the 400+ packages on PyPI which claim to be Py3 compatible, into a virtual env, using "pysetup3 install". I then noticed some (slight) performance slowdown and sudden disappearance of free disk space ... it was all those archives (and their unpacked contents) in /tmp that was the reason.
msg139794 - (view)	Author: Alexis Metaireau (alexis) *	Date: 2011-07-04 17:19
I'm +1 on applying this patch as well. Removing files in the tmp directory is far better than letting the OS doing so.
msg139971 - (view)	Author: Roundup Robot (python-dev)	Date: 2011-07-07 11:59
New changeset a4405b799e1b by Vinay Sajip in branch 'default': Closes #12391: temporary files are now cleaned up. http://hg.python.org/cpython/rev/a4405b799e1b