[Python-checkins] cpython (merge 3.2 -> default): merge #11767: use context manager to close file in __getitem__ to prevent FD

[r.david.murray]

http://hg.python.org/cpython/rev/1d7a91358517
changeset:   70844:1d7a91358517
parent:      70842:c9d27c63b45c
parent:      70843:fea1920ae75f
user:        R David Murray <rdmurray at bitdance.com>
date:        Fri Jun 17 12:56:41 2011 -0400
summary:
  merge #11767: use context manager to close file in __getitem__ to prevent FD leak

files:
  Lib/mailbox.py           |   4 ++-
  Lib/test/test_mailbox.py |  33 +++++++++++++++++++++++++++-
  Misc/NEWS                |   2 +
  3 files changed, 37 insertions(+), 2 deletions(-)


diff --git a/Lib/mailbox.py b/Lib/mailbox.py
--- a/Lib/mailbox.py
+++ b/Lib/mailbox.py
@@ -20,6 +20,7 @@
 import email.message
 import email.generator
 import io
+import contextlib
 try:
     if sys.platform == 'os2emx':
         # OS/2 EMX fcntl() not adequate
@@ -76,7 +77,8 @@
         if not self._factory:
             return self.get_message(key)
         else:
-            return self._factory(self.get_file(key))
+            with contextlib.closing(self.get_file(key)) as file:
+                return self._factory(file)
 
     def get_message(self, key):
         """Return a Message representation or raise a KeyError."""
diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py
--- a/Lib/test/test_mailbox.py
+++ b/Lib/test/test_mailbox.py
@@ -1201,6 +1201,37 @@
         self.assertEqual(set(self._box.get_labels()), set(['blah']))
 
 
+class FakeFileLikeObject:
+
+    def __init__(self):
+        self.closed = False
+
+    def close(self):
+        self.closed = True
+
+
+class FakeMailBox(mailbox.Mailbox):
+
+    def __init__(self):
+        mailbox.Mailbox.__init__(self, '', lambda file: None)
+        self.files = [FakeFileLikeObject() for i in range(10)]
+
+    def get_file(self, key):
+        return self.files[key]
+
+
+class TestFakeMailBox(unittest.TestCase):
+
+    def test_closing_fd(self):
+        box = FakeMailBox()
+        for i in range(10):
+            self.assertFalse(box.files[i].closed)
+        for i in range(10):
+            box[i]
+        for i in range(10):
+            self.assertTrue(box.files[i].closed)
+
+
 class TestMessage(TestBase):
 
     _factory = mailbox.Message      # Overridden by subclasses to reuse tests
@@ -2113,7 +2144,7 @@
              TestBabyl, TestMessage, TestMaildirMessage, TestMboxMessage,
              TestMHMessage, TestBabylMessage, TestMMDFMessage,
              TestMessageConversion, TestProxyFile, TestPartialFile,
-             MaildirTestCase)
+             MaildirTestCase, TestFakeMailBox)
     support.run_unittest(*tests)
     support.reap_children()
 
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -193,6 +193,8 @@
 Library
 -------
 
+- Issue #11767: Correct file descriptor leak in mailbox's __getitem__ method.
+
 - Issue #12133: AbstractHTTPHandler.do_open() of urllib.request closes the HTTP
   connection if its getresponse() method fails with a socket error. Patch
   written by Ezio Melotti.

-- 
Repository URL: http://hg.python.org/cpython