From 1f99f9d5c2ebe35838e275b3b4d4bb96ab212def Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 25 Mar 2014 09:18:04 +0100 Subject: [PATCH] Issue #21058: Fix a leak of file descriptor in tempfile.NamedTemporaryFile(), close the file descriptor if io.open() fails --- Lib/tempfile.py | 10 +++++++--- Lib/test/test_tempfile.py | 24 ++++++++++++++++++++++++ Misc/NEWS | 4 ++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 57e8a829ec0..f0e25fc2d54 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -458,10 +458,14 @@ def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, flags |= _os.O_TEMPORARY (fd, name) = _mkstemp_inner(dir, prefix, suffix, flags) - file = _io.open(fd, mode, buffering=buffering, - newline=newline, encoding=encoding) + try: + file = _io.open(fd, mode, buffering=buffering, + newline=newline, encoding=encoding) - return _TemporaryFileWrapper(file, name, delete) + return _TemporaryFileWrapper(file, name, delete) + except Exception: + _os.close(fd) + raise if _os.name != 'posix' or _os.sys.platform == 'cygwin': # On non-POSIX and Cygwin systems, assume that we cannot unlink a file diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index cf2ae080bed..fda914e2a3c 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -9,6 +9,7 @@ import re import warnings import contextlib import weakref +from unittest import mock import unittest from test import support, script_helper @@ -758,6 +759,17 @@ class TestNamedTemporaryFile(BaseTestCase): pass self.assertRaises(ValueError, use_closed) + def test_no_leak_fd(self): + # Issue #21058: don't leak file descriptor when io.pen() fails + closed = [] + def close(fd): + closed.append(fd) + + with mock.patch('os.close', side_effect=close): + with mock.patch('io.open', side_effect=ValueError): + self.assertRaises(ValueError, tempfile.NamedTemporaryFile) + self.assertEqual(len(closed), 1) + # How to test the mode and bufsize parameters? @@ -1061,6 +1073,18 @@ if tempfile.NamedTemporaryFile is not tempfile.TemporaryFile: roundtrip("\u039B", "w+", encoding="utf-16") roundtrip("foo\r\n", "w+", newline="") + def test_no_leak_fd(self): + # Issue #21058: don't leak file descriptor when io.open() fails + closed = [] + def close(fd): + closed.append(fd) + + with mock.patch('os.close', side_effect=close): + with mock.patch('io.open', side_effect=ValueError): + self.assertRaises(ValueError, tempfile.TemporaryFile) + self.assertEqual(len(closed), 1) + + # Helper for test_del_on_shutdown class NulledModules: diff --git a/Misc/NEWS b/Misc/NEWS index d1fbd4b06a1..2a7cd3bf9e2 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -24,6 +24,10 @@ Core and Builtins Library ------- +- Issue #21058: Fix a leak of file descriptor in + :func:`tempfile.NamedTemporaryFile`, close the file descriptor if + :func:`io.open` fails + - Issue #21013: Enhance ssl.create_default_context() when used for server side sockets to provide better security by default.