bpo-38076 Clear the interpreter state only after clearing module globals (GH-18039)
Currently, during runtime destruction, `_PyImport_Cleanup` is clearing the interpreter state before clearing out the modules themselves. This leads to a segfault on modules that rely on the module state to clear themselves up.
For example, let's take the small snippet added in the issue by @DinoV :
```
import _struct
class C:
def __init__(self):
self.pack = _struct.pack
def __del__(self):
self.pack('I', -42)
_struct.x = C()
```
The module `_struct` uses the module state to run `pack`. Therefore, the module state has to be alive until after the module has been cleared out to successfully run `C.__del__`. This happens at line 606, when `_PyImport_Cleanup` calls `_PyModule_Clear`. In fact, the loop that calls `_PyModule_Clear` has in its comments:
> Now, if there are any modules left alive, clear their globals to minimize potential leaks. All C extension modules actually end up here, since they are kept alive in the interpreter state.
That means that we can't clear the module state (which is used by C Extensions) before we run that loop.
Moving `_PyInterpreterState_ClearModules` until after it, fixes the segfault in the code snippet.
Finally, this updates a test in `io` to correctly assert the error that it now throws (since it now finds the io module state). The test that uses this is: `test_create_at_shutdown_without_encoding`. Given this test is now working is a proof that the module state now stays alive even when `__del__` is called at module destruction time. Thus, I didn't add a new tests for this.
https://bugs.python.org/issue38076diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index 8a123fa..a66726e 100644
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -3683,7 +3683,7 @@
class CTextIOWrapperTest(TextIOWrapperTest):
io = io
- shutdown_error = "RuntimeError: could not find io module state"
+ shutdown_error = "LookupError: unknown encoding: ascii"
def test_initialization(self):
r = self.BytesIO(b"\xc3\xa9\n\n")
diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py
index 157efa1..4829fbe 100644
--- a/Lib/test/test_struct.py
+++ b/Lib/test/test_struct.py
@@ -7,6 +7,7 @@
import sys
from test import support
+from test.support.script_helper import assert_python_ok
ISBIGENDIAN = sys.byteorder == "big"
@@ -652,6 +653,23 @@
s2 = struct.Struct(s.format.encode())
self.assertEqual(s2.format, s.format)
+ def test_struct_cleans_up_at_runtime_shutdown(self):
+ code = """if 1:
+ import struct
+
+ class C:
+ def __init__(self):
+ self.pack = struct.pack
+ def __del__(self):
+ self.pack('I', -42)
+
+ struct.x = C()
+ """
+ rc, stdout, stderr = assert_python_ok("-c", code)
+ self.assertEqual(rc, 0)
+ self.assertEqual(stdout.rstrip(), b"")
+ self.assertIn(b"Exception ignored in:", stderr)
+ self.assertIn(b"C.__del__", stderr)
class UnpackIteratorTest(unittest.TestCase):
"""
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
index 58701a1..c5bd8a4 100644
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -855,6 +855,23 @@
self.assertIn(b'sys.flags', out[0])
self.assertIn(b'sys.float_info', out[1])
+ def test_sys_ignores_cleaning_up_user_data(self):
+ code = """if 1:
+ import struct, sys
+
+ class C:
+ def __init__(self):
+ self.pack = struct.pack
+ def __del__(self):
+ self.pack('I', -42)
+
+ sys.x = C()
+ """
+ rc, stdout, stderr = assert_python_ok('-c', code)
+ self.assertEqual(rc, 0)
+ self.assertEqual(stdout.rstrip(), b"")
+ self.assertEqual(stderr.rstrip(), b"")
+
@unittest.skipUnless(hasattr(sys, 'getandroidapilevel'),
'need sys.getandroidapilevel()')
def test_getandroidapilevel(self):