[3.6] bpo-29110: Fix file object leak in `aifc.open` (#310)
(cherry picked from commit 03f68b60e17b57f6f13729ff73245dbb37b30a4c) (GH-162)
(cherry picked from commit 5dc33eea538361f8a218255f83db2e9298dd8c53) (GH-293)
diff --git a/Lib/aifc.py b/Lib/aifc.py
index 692d0bf..13ad7dc 100644
--- a/Lib/aifc.py
+++ b/Lib/aifc.py
@@ -303,6 +303,8 @@
# _ssnd_chunk -- instantiation of a chunk class for the SSND chunk
# _framesize -- size of one frame in the file
+ _file = None # Set here since __del__ checks it
+
def initfp(self, file):
self._version = 0
self._convert = None
@@ -344,9 +346,15 @@
def __init__(self, f):
if isinstance(f, str):
- f = builtins.open(f, 'rb')
- # else, assume it is an open file object already
- self.initfp(f)
+ file_object = builtins.open(f, 'rb')
+ try:
+ self.initfp(file_object)
+ except:
+ file_object.close()
+ raise
+ else:
+ # assume it is an open file object already
+ self.initfp(f)
def __enter__(self):
return self
@@ -541,18 +549,23 @@
# _datalength -- the size of the audio samples written to the header
# _datawritten -- the size of the audio samples actually written
+ _file = None # Set here since __del__ checks it
+
def __init__(self, f):
if isinstance(f, str):
- filename = f
- f = builtins.open(f, 'wb')
+ file_object = builtins.open(f, 'wb')
+ try:
+ self.initfp(file_object)
+ except:
+ file_object.close()
+ raise
+
+ # treat .aiff file extensions as non-compressed audio
+ if f.endswith('.aiff'):
+ self._aifc = 0
else:
- # else, assume it is an open file object already
- filename = '???'
- self.initfp(f)
- if filename[-5:] == '.aiff':
- self._aifc = 0
- else:
- self._aifc = 1
+ # assume it is an open file object already
+ self.initfp(f)
def initfp(self, file):
self._file = file
diff --git a/Lib/test/test_aifc.py b/Lib/test/test_aifc.py
index 1bd1f89..a731a51 100644
--- a/Lib/test/test_aifc.py
+++ b/Lib/test/test_aifc.py
@@ -1,5 +1,6 @@
-from test.support import findfile, TESTFN, unlink
+from test.support import check_no_resource_warning, findfile, TESTFN, unlink
import unittest
+from unittest import mock
from test import audiotests
from audioop import byteswap
import io
@@ -149,6 +150,21 @@
#This file contains chunk types aifc doesn't recognize.
self.f = aifc.open(findfile('Sine-1000Hz-300ms.aif'))
+ def test_close_opened_files_on_error(self):
+ non_aifc_file = findfile('pluck-pcm8.wav', subdir='audiodata')
+ with check_no_resource_warning(self):
+ with self.assertRaises(aifc.Error):
+ # Try opening a non-AIFC file, with the expectation that
+ # `aifc.open` will fail (without raising a ResourceWarning)
+ self.f = aifc.open(non_aifc_file, 'rb')
+
+ # Aifc_write.initfp() won't raise in normal case. But some errors
+ # (e.g. MemoryError, KeyboardInterrupt, etc..) can happen.
+ with mock.patch.object(aifc.Aifc_write, 'initfp',
+ side_effect=RuntimeError):
+ with self.assertRaises(RuntimeError):
+ self.fout = aifc.open(TESTFN, 'wb')
+
def test_params_added(self):
f = self.f = aifc.open(TESTFN, 'wb')
f.aiff()
diff --git a/Misc/NEWS b/Misc/NEWS
index dfafe7b..5fad3c8 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -78,6 +78,9 @@
- bpo-29532: Altering a kwarg dictionary passed to functools.partial()
no longer affects a partial object after creation.
+- bpo-29110: Fix file object leak in aifc.open() when file is given as a
+ filesystem path and is not in valid AIFF format. Patch by Anthony Zhang.
+
- bpo-22807: Add uuid.SafeUUID and uuid.UUID.is_safe to relay information from
the platform about whether generated UUIDs are generated with a
multiprocessing safe method.