bpo-18748: io.IOBase destructor now logs close() errors in dev mode (GH-12786)

vstinner · web-flow · commit 44235041f3b9 · 2019-04-12T17:06:47.000+02:00
In development mode (-X dev) and in debug build, the io.IOBase
destructor now logs close() exceptions. These exceptions are silent
by default in release mode.
diff --git a/Doc/using/cmdline.rst b/Doc/using/cmdline.rst
@@ -437,6 +437,7 @@ Miscellaneous options
      * Enable :ref:`asyncio debug mode <asyncio-debug-mode>`.
      * Set the :attr:`~sys.flags.dev_mode` attribute of :attr:`sys.flags` to
        ``True``
+     * :class:`io.IOBase` destructor logs ``close()`` exceptions.
 
    * ``-X utf8`` enables UTF-8 mode for operating system interfaces, overriding
      the default locale-aware mode. ``-X utf8=0`` explicitly disables UTF-8
@@ -465,7 +466,8 @@ Miscellaneous options
       The ``-X importtime``, ``-X dev`` and ``-X utf8`` options.
 
    .. versionadded:: 3.8
-      The ``-X pycache_prefix`` option.
+      The ``-X pycache_prefix`` option. The ``-X dev`` option now logs
+      ``close()`` exceptions in :class:`io.IOBase` destructor.
 
 
 Options you shouldn't use
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
@@ -67,6 +67,11 @@ class EmptyStruct(ctypes.Structure):
     '--with-memory-sanitizer' in _config_args
 )
 
+# Does io.IOBase logs unhandled exceptions on calling close()?
+# They are silenced by default in release build.
+DESTRUCTOR_LOG_ERRORS = (hasattr(sys, "gettotalrefcount") or sys.flags.dev_mode)
+
+
 def _default_chunk_size():
     """Get the default TextIOWrapper chunk size"""
     with open(__file__, "r", encoding="latin-1") as f:
@@ -1097,9 +1102,16 @@ def f():
         s = s.getvalue().strip()
         if s:
             # The destructor *may* have printed an unraisable error, check it
-            self.assertEqual(len(s.splitlines()), 1)
-            self.assertTrue(s.startswith("Exception OSError: "), s)
-            self.assertTrue(s.endswith(" ignored"), s)
+            lines = s.splitlines()
+            if DESTRUCTOR_LOG_ERRORS:
+                self.assertEqual(len(lines), 5)
+                self.assertTrue(lines[0].startswith("Exception ignored in: "), lines)
+                self.assertEqual(lines[1], "Traceback (most recent call last):", lines)
+                self.assertEqual(lines[4], 'OSError:', lines)
+            else:
+                self.assertEqual(len(lines), 1)
+                self.assertTrue(lines[-1].startswith("Exception OSError: "), lines)
+                self.assertTrue(lines[-1].endswith(" ignored"), lines)
 
     def test_repr(self):
         raw = self.MockRawIO()
@@ -2833,9 +2845,16 @@ def f():
         s = s.getvalue().strip()
         if s:
             # The destructor *may* have printed an unraisable error, check it
-            self.assertEqual(len(s.splitlines()), 1)
-            self.assertTrue(s.startswith("Exception OSError: "), s)
-            self.assertTrue(s.endswith(" ignored"), s)
+            lines = s.splitlines()
+            if DESTRUCTOR_LOG_ERRORS:
+                self.assertEqual(len(lines), 5)
+                self.assertTrue(lines[0].startswith("Exception ignored in: "), lines)
+                self.assertEqual(lines[1], "Traceback (most recent call last):", lines)
+                self.assertEqual(lines[4], 'OSError:', lines)
+            else:
+                self.assertEqual(len(lines), 1)
+                self.assertTrue(lines[-1].startswith("Exception OSError: "), lines)
+                self.assertTrue(lines[-1].endswith(" ignored"), lines)
 
     # Systematic tests of the text I/O API
 
diff --git a/Misc/NEWS.d/next/Library/2019-04-11-16-09-42.bpo-18748.QW7upB.rst b/Misc/NEWS.d/next/Library/2019-04-11-16-09-42.bpo-18748.QW7upB.rst
@@ -0,0 +1,3 @@
+In development mode (:option:`-X` ``dev``) and in debug build, the
+:class:`io.IOBase` destructor now logs ``close()`` exceptions. These exceptions
+are silent by default in release mode.
diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c
@@ -286,10 +286,22 @@ iobase_finalize(PyObject *self)
         /* Silencing I/O errors is bad, but printing spurious tracebacks is
            equally as bad, and potentially more frequent (because of
            shutdown issues). */
-        if (res == NULL)
-            PyErr_Clear();
-        else
+        if (res == NULL) {
+#ifndef Py_DEBUG
+            const _PyCoreConfig *config = &_PyInterpreterState_GET_UNSAFE()->core_config;
+            if (config->dev_mode) {
+                PyErr_WriteUnraisable(self);
+            }
+            else {
+                PyErr_Clear();
+            }
+#else
+            PyErr_WriteUnraisable(self);
+#endif
+        }
+        else {
             Py_DECREF(res);
+        }
     }
 
     /* Restore the saved exception. */

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+In development mode (:option:`-X` ``dev``) and in debug build, the
	`2`	+:class:`io.IOBase` destructor now logs ``close()`` exceptions. These exceptions
	`3`	`+are silent by default in release mode.`