[issue16485] FD leaks in aifc module
Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Roundup Robot added the comment: New changeset cf8d692cc847 by Serhiy Storchaka in branch '2.7': Issue #16485: Fix file descriptor not being closed if file header patching fails on closing of aifc file. http://hg.python.org/cpython/rev/cf8d692cc847 New changeset e3c4e9f4ea0f by Serhiy Storchaka in branch '3.2': Issue #16485: Fix file descriptor not being closed if file header patching fails on closing of aifc file. http://hg.python.org/cpython/rev/e3c4e9f4ea0f New changeset 9a571c4a16d1 by Serhiy Storchaka in branch '3.3': Issue #16485: Fix file descriptor not being closed if file header patching fails on closing of aifc file. http://hg.python.org/cpython/rev/9a571c4a16d1 New changeset 79a8f6e1dfb0 by Serhiy Storchaka in branch 'default': Issue #16485: Fix file descriptor not being closed if file header patching fails on closing of aifc file. http://hg.python.org/cpython/rev/79a8f6e1dfb0 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: -- resolution: - works for me stage: patch review - committed/rejected status: open - closed versions: +Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: -- resolution: works for me - fixed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Serhiy Storchaka added the comment: Of course. Thanks for point. It's my editor made wrong whitespace changes after block indent/unindent. hg diff shows this changes and I shouldn't miss this. I will review every patch before commit one more time. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Serhiy Storchaka added the comment: If no one objects I will commit this next year. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: -- assignee: - serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Georg Brandl added the comment: Looks good to me, except: the patch contains unrelated whitespace changes. Please don't commit them along the fix. If you think they are really necessary, they should go in a separate commit. -- nosy: +georg.brandl ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
New submission from Serhiy Storchaka: Aifc_write.close() can raise exception and left the internal file object not closed. The patch closes the file object even in case of error and reset _file to None even in case of the file object close() raises an exception, so that Aifc_write.close() can be called repeatedly. -- components: Library (Lib) files: aifc_close.patch keywords: patch messages: 175683 nosy: serhiy.storchaka priority: normal severity: normal status: open title: FD leaks in aifc module type: resource usage versions: Python 3.2, Python 3.3, Python 3.4 Added file: http://bugs.python.org/file27996/aifc_close.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: Removed file: http://bugs.python.org/file27996/aifc_close.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: -- nosy: +r.david.murray Added file: http://bugs.python.org/file27997/aifc_close.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: Added file: http://bugs.python.org/file27998/aifc_close-2.7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Ezio Melotti added the comment: Adding with support to Aifc_write looks like a new feature, but Aifc_write doesn't seem to be part of the public API. Does this change (indirectly) add with support to any part of the public API? -- nosy: +ezio.melotti stage: - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Ezio Melotti added the comment: Adding with support to Aifc_write looks like a new feature, but Aifc_write doesn't seem to be part of the public API. Does this change (indirectly) add with support to any part of the public API? -- nosy: +ezio.melotti stage: - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: Removed file: http://bugs.python.org/file27998/aifc_close-2.7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Serhiy Storchaka added the comment: Sorry, I miss that test_close() already exists. Merged. -- Added file: http://bugs.python.org/file27999/aifc_close.patch Added file: http://bugs.python.org/file28000/aifc_close-2.7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___diff -r cb612c5f30cb Lib/aifc.py --- a/Lib/aifc.py Thu Nov 15 18:22:23 2012 + +++ b/Lib/aifc.py Fri Nov 16 15:22:51 2012 +0200 @@ -692,7 +692,9 @@ self._patchheader() def close(self): -if self._file: +if self._file is None: +return +try: self._ensure_header_written(0) if self._datawritten 1: # quick pad to even size @@ -700,13 +702,15 @@ self._datawritten = self._datawritten + 1 self._writemarkers() if self._nframeswritten != self._nframes or \ - self._datalength != self._datawritten or \ - self._marklength: +self._datalength != self._datawritten or \ +self._marklength: self._patchheader() +finally: # Prevent ref cycles self._convert = None -self._file.close() +f = self._file self._file = None +f.close() # # Internal methods. diff -r cb612c5f30cb Lib/test/test_aifc.py --- a/Lib/test/test_aifc.py Thu Nov 15 18:22:23 2012 + +++ b/Lib/test/test_aifc.py Fri Nov 16 15:22:51 2012 +0200 @@ -112,6 +112,13 @@ self.assertEqual(testfile.closed, False) f.close() self.assertEqual(testfile.closed, True) +testfile = open(TESTFN, 'wb') +fout = aifc.open(testfile, 'wb') +self.assertFalse(testfile.closed) +with self.assertRaises(aifc.Error): +fout.close() +self.assertTrue(testfile.closed) +fout.close() # do nothing def test_write_header_comptype_sampwidth(self): for comptype in (b'ULAW', b'ulaw', b'ALAW', b'alaw', b'G722'): @@ -286,11 +293,13 @@ def test_write_header_raises(self): fout = aifc.open(io.BytesIO(), 'wb') self.assertRaises(aifc.Error, fout.close) +fout = aifc.open(io.BytesIO(), 'wb') fout.setnchannels(1) self.assertRaises(aifc.Error, fout.close) +fout = aifc.open(io.BytesIO(), 'wb') +fout.setnchannels(1) fout.setsampwidth(1) self.assertRaises(aifc.Error, fout.close) -fout.initfp(None) def test_write_header_comptype_raises(self): for comptype in (b'ULAW', b'ulaw', b'ALAW', b'alaw', b'G722'): diff -r 457c0c9c7893 Lib/aifc.py --- a/Lib/aifc.py Thu Nov 15 07:10:27 2012 +0100 +++ b/Lib/aifc.py Fri Nov 16 15:25:05 2012 +0200 @@ -732,22 +732,28 @@ self._patchheader() def close(self): -self._ensure_header_written(0) -if self._datawritten 1: -# quick pad to even size -self._file.write(chr(0)) -self._datawritten = self._datawritten + 1 -self._writemarkers() -if self._nframeswritten != self._nframes or \ - self._datalength != self._datawritten or \ - self._marklength: -self._patchheader() -if self._comp: -self._comp.CloseCompressor() -self._comp = None -# Prevent ref cycles -self._convert = None -self._file.close() +if self._file is None: +return +try: +self._ensure_header_written(0) +if self._datawritten 1: +# quick pad to even size +self._file.write(chr(0)) +self._datawritten = self._datawritten + 1 +self._writemarkers() +if self._nframeswritten != self._nframes or \ +self._datalength != self._datawritten or \ +self._marklength: +self._patchheader() +if self._comp: +self._comp.CloseCompressor() +self._comp = None +finally: +# Prevent ref cycles +self._convert = None +f = self._file +self._file = None +f.close() # # Internal methods. diff -r 457c0c9c7893 Lib/test/test_aifc.py --- a/Lib/test/test_aifc.py Thu Nov 15 07:10:27 2012 +0100 +++ b/Lib/test/test_aifc.py Fri Nov 16 15:25:05 2012 +0200 @@ -106,6 +106,13 @@ self.assertEqual(testfile.closed, False) f.close() self.assertEqual(testfile.closed, True) +testfile = open(TESTFN, 'wb') +fout = aifc.open(testfile, 'wb') +self.assertFalse(testfile.closed) +with self.assertRaises(aifc.Error): +fout.close() +self.assertTrue(testfile.closed) +fout.close() # do nothing
[issue16485] FD leaks in aifc module
Changes by Serhiy Storchaka storch...@gmail.com: Removed file: http://bugs.python.org/file27997/aifc_close.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16485] FD leaks in aifc module
Serhiy Storchaka added the comment: Adding with support to Aifc_write looks like a new feature, but Aifc_write doesn't seem to be part of the public API. Does this change (indirectly) add with support to any part of the public API? Don't pay attention, it was a wrong patch. For with support see issue16486. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16485 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
