[Python-checkins] bpo-26789: Fix logging.FileHandler._open() at exit (GH-23053)

[vstinner]

https://github.com/python/cpython/commit/45df61fd2d58e8db33179f3b5d00e53fe6a7e592
commit: 45df61fd2d58e8db33179f3b5d00e53fe6a7e592
branch: master
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2020-11-02T23:17:46+01:00
summary:

bpo-26789: Fix logging.FileHandler._open() at exit (GH-23053)

The logging.FileHandler class now keeps a reference to the builtin
open() function to be able to open or reopen the file during Python
finalization.

Fix errors like:

    Exception ignored in: (...)
    Traceback (most recent call last):
      (...)
      File ".../logging/__init__.py", line 1463, in error
      File ".../logging/__init__.py", line 1577, in _log
      File ".../logging/__init__.py", line 1587, in handle
      File ".../logging/__init__.py", line 1649, in callHandlers
      File ".../logging/__init__.py", line 948, in handle
      File ".../logging/__init__.py", line 1182, in emit
      File ".../logging/__init__.py", line 1171, in _open
    NameError: name 'open' is not defined

files:
A Misc/NEWS.d/next/Library/2020-10-31-01-16-49.bpo-26789.9BdNAt.rst
M Lib/logging/__init__.py
M Lib/test/test_logging.py
M Python/_warnings.c

diff --git a/Lib/logging/__init__.py b/Lib/logging/__init__.py
index 265e286101e91..badfd654b1689 100644
--- a/Lib/logging/__init__.py
+++ b/Lib/logging/__init__.py
@@ -1148,6 +1148,10 @@ def __init__(self, filename, mode='a', encoding=None, delay=False, errors=None):
         self.encoding = encoding
         self.errors = errors
         self.delay = delay
+        # bpo-26789: FileHandler keeps a reference to the builtin open()
+        # function to be able to open or reopen the file during Python
+        # finalization.
+        self._builtin_open = open
         if delay:
             #We don't open the stream, but we still need to call the
             #Handler constructor to set level, formatter, lock etc.
@@ -1183,8 +1187,9 @@ def _open(self):
         Open the current base file with the (original) mode and encoding.
         Return the resulting stream.
         """
-        return open(self.baseFilename, self.mode, encoding=self.encoding,
-                    errors=self.errors)
+        open_func = self._builtin_open
+        return open_func(self.baseFilename, self.mode,
+                         encoding=self.encoding, errors=self.errors)
 
     def emit(self, record):
         """
diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py
index 7c98e19b7408f..e2196736dcdf4 100644
--- a/Lib/test/test_logging.py
+++ b/Lib/test/test_logging.py
@@ -4310,8 +4310,8 @@ def __init__(self, name='MyLogger', level=logging.NOTSET):
             logging.setLoggerClass(logging.Logger)
 
     def test_logging_at_shutdown(self):
-        # Issue #20037
-        code = """if 1:
+        # bpo-20037: Doing text I/O late at interpreter shutdown must not crash
+        code = textwrap.dedent("""
             import logging
 
             class A:
@@ -4321,22 +4321,55 @@ def __del__(self):
                     except Exception:
                         logging.exception("exception in __del__")
 
-            a = A()"""
+            a = A()
+        """)
         rc, out, err = assert_python_ok("-c", code)
         err = err.decode()
         self.assertIn("exception in __del__", err)
         self.assertIn("ValueError: some error", err)
 
+    def test_logging_at_shutdown_open(self):
+        # bpo-26789: FileHandler keeps a reference to the builtin open()
+        # function to be able to open or reopen the file during Python
+        # finalization.
+        filename = os_helper.TESTFN
+        self.addCleanup(os_helper.unlink, filename)
+
+        code = textwrap.dedent(f"""
+            import builtins
+            import logging
+
+            class A:
+                def __del__(self):
+                    logging.error("log in __del__")
+
+            # basicConfig() opens the file, but logging.shutdown() closes
+            # it at Python exit. When A.__del__() is called,
+            # FileHandler._open() must be called again to re-open the file.
+            logging.basicConfig(filename={filename!r})
+
+            a = A()
+
+            # Simulate the Python finalization which removes the builtin
+            # open() function.
+            del builtins.open
+        """)
+        assert_python_ok("-c", code)
+
+        with open(filename) as fp:
+            self.assertEqual(fp.read().rstrip(), "ERROR:root:log in __del__")
+
     def test_recursion_error(self):
         # Issue 36272
-        code = """if 1:
+        code = textwrap.dedent("""
             import logging
 
             def rec():
                 logging.error("foo")
                 rec()
 
-            rec()"""
+            rec()
+        """)
         rc, out, err = assert_python_failure("-c", code)
         err = err.decode()
         self.assertNotIn("Cannot recover from stack overflow.", err)
diff --git a/Misc/NEWS.d/next/Library/2020-10-31-01-16-49.bpo-26789.9BdNAt.rst b/Misc/NEWS.d/next/Library/2020-10-31-01-16-49.bpo-26789.9BdNAt.rst
new file mode 100644
index 0000000000000..d8832401c932b
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-10-31-01-16-49.bpo-26789.9BdNAt.rst
@@ -0,0 +1,4 @@
+The :class:`logging.FileHandler` class now keeps a reference to the builtin
+:func:`open` function to be able to open or reopen the file during Python
+finalization. Fix errors like: ``NameError: name 'open' is not defined``. Patch
+by Victor Stinner.
diff --git a/Python/_warnings.c b/Python/_warnings.c
index 271cd47f4eee6..3c048af4193a0 100644
--- a/Python/_warnings.c
+++ b/Python/_warnings.c
@@ -852,7 +852,7 @@ setup_context(Py_ssize_t stack_level, PyObject **filename, int *lineno,
     }
 
     if (f == NULL) {
-        globals = _PyInterpreterState_GET()->sysdict;
+        globals = tstate->interp->sysdict;
         *filename = PyUnicode_FromString("sys");
         *lineno = 1;
     }