Issue #18879: When a method is looked up on a temporary file, avoid closing the file...

diff --git a/Lib/tempfile.py b/Lib/tempfile.py
index 91332b61be72e605b1dd36deaef5a3c789329a21..b4b5c88338c35551c606a50d2d89f93284ae4961 100644 (file)
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -27,6 +27,7 @@ __all__ = [
 
 # Imports.
 
+import functools as _functools
 import warnings as _warnings
 import sys as _sys
 import io as _io
@@ -349,6 +350,46 @@ def mktemp(suffix="", prefix=template, dir=None):
                           "No usable temporary filename found")
 
 
+class _TemporaryFileCloser:
+    """A separate object allowing proper closing of a temporary file's
+    underlying file object, without adding a __del__ method to the
+    temporary file."""
+
+    def __init__(self, file, name, delete=True):
+        self.file = file
+        self.name = name
+        self.close_called = False
+        self.delete = delete
+
+    # NT provides delete-on-close as a primitive, so we don't need
+    # the wrapper to do anything special.  We still use it so that
+    # file.name is useful (i.e. not "(fdopen)") with NamedTemporaryFile.
+    if _os.name != 'nt':
+        # Cache the unlinker so we don't get spurious errors at
+        # shutdown when the module-level "os" is None'd out.  Note
+        # that this must be referenced as self.unlink, because the
+        # name TemporaryFileWrapper may also get None'd out before
+        # __del__ is called.
+        unlink = _os.unlink
+
+        def close(self):
+            if not self.close_called:
+                self.close_called = True
+                self.file.close()
+                if self.delete:
+                    self.unlink(self.name)
+
+        # Need to ensure the file is deleted on __del__
+        def __del__(self):
+            self.close()
+
+    else:
+        def close(self):
+            if not self.close_called:
+                self.close_called = True
+                self.file.close()
+
+
 class _TemporaryFileWrapper:
     """Temporary file wrapper
 
@@ -360,8 +401,8 @@ class _TemporaryFileWrapper:
     def __init__(self, file, name, delete=True):
         self.file = file
         self.name = name
-        self.close_called = False
         self.delete = delete
+        self._closer = _TemporaryFileCloser(file, name, delete)
 
     def __getattr__(self, name):
         # Attribute lookups are delegated to the underlying file
@@ -369,6 +410,15 @@ class _TemporaryFileWrapper:
         # (i.e. methods are cached, closed and friends are not)
         file = self.__dict__['file']
         a = getattr(file, name)
+        if hasattr(a, '__call__'):
+            func = a
+            @_functools.wraps(func)
+            def func_wrapper(*args, **kwargs):
+                return func(*args, **kwargs)
+            # Avoid closing the file as long as the wrapper is alive,
+            # see issue #18879.
+            func_wrapper._closer = self._closer
+            a = func_wrapper
         if not isinstance(a, int):
             setattr(self, name, a)
         return a
@@ -379,41 +429,23 @@ class _TemporaryFileWrapper:
         self.file.__enter__()
         return self
 
+    # Need to trap __exit__ as well to ensure the file gets
+    # deleted when used in a with statement
+    def __exit__(self, exc, value, tb):
+        result = self.file.__exit__(exc, value, tb)
+        self.close()
+        return result
+
+    def close(self):
+        """
+        Close the temporary file, possibly deleting it.
+        """
+        self._closer.close()
+
     # iter() doesn't use __getattr__ to find the __iter__ method
     def __iter__(self):
         return iter(self.file)
 
-    # NT provides delete-on-close as a primitive, so we don't need
-    # the wrapper to do anything special.  We still use it so that
-    # file.name is useful (i.e. not "(fdopen)") with NamedTemporaryFile.
-    if _os.name != 'nt':
-        # Cache the unlinker so we don't get spurious errors at
-        # shutdown when the module-level "os" is None'd out.  Note
-        # that this must be referenced as self.unlink, because the
-        # name TemporaryFileWrapper may also get None'd out before
-        # __del__ is called.
-        unlink = _os.unlink
-
-        def close(self):
-            if not self.close_called:
-                self.close_called = True
-                self.file.close()
-                if self.delete:
-                    self.unlink(self.name)
-
-        def __del__(self):
-            self.close()
-
-        # Need to trap __exit__ as well to ensure the file gets
-        # deleted when used in a with statement
-        def __exit__(self, exc, value, tb):
-            result = self.file.__exit__(exc, value, tb)
-            self.close()
-            return result
-    else:
-        def __exit__(self, exc, value, tb):
-            self.file.__exit__(exc, value, tb)
-
 
 def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None,
                        newline=None, suffix="", prefix=template,

diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py
index 9c4c158b22bd8771fec14302848f630aa84eae54..e708ce88ce108fc9c91512e0c9973f72d30db0f5 100644 (file)
--- a/Lib/test/test_tempfile.py
+++ b/Lib/test/test_tempfile.py
@@ -8,6 +8,7 @@ import sys
 import re
 import warnings
 import contextlib
+import weakref
 
 import unittest
 from test import support
@@ -674,6 +675,22 @@ class TestNamedTemporaryFile(BaseTestCase):
         self.do_create(pre="a", suf="b")
         self.do_create(pre="aa", suf=".txt")
 
+    def test_method_lookup(self):
+        # Issue #18879: Looking up a temporary file method should keep it
+        # alive long enough.
+        f = self.do_create()
+        wr = weakref.ref(f)
+        write = f.write
+        write2 = f.write
+        del f
+        write(b'foo')
+        del write
+        write2(b'bar')
+        del write2
+        if support.check_impl_detail(cpython=True):
+            # No reference cycle was created.
+            self.assertIsNone(wr())
+
     def test_creates_named(self):
         # NamedTemporaryFile creates files with names
         f = tempfile.NamedTemporaryFile()

diff --git a/Misc/NEWS b/Misc/NEWS
index 80c284c17e5b91f68f3798a7413442d7e7f0923b..61c4482490d10fd3673ca13543e2a8670b4efea0 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -29,6 +29,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #18879: When a method is looked up on a temporary file, avoid closing
+  the file before the method is possibly called.
+
 - Issue #20034: Updated alias mapping to most recent locale.alias file
   from X.org distribution using makelocalealias.py.