gh-115490: Make the interpreter.channels and interpreter.queues Modules Handle Reload...

The problem manifested when the .py module got reloaded and the corresponding extension module didn't. The .py module registers types with the extension and the extension was not allowing that to happen more than once. The solution: let it happen more than once.

diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py
index 6fbe3d00981ddd2dea233facbcf27c555889211e..126daca388fd7f13125fdd52b1918af89f58188b 100644 (file)
--- a/Lib/test/libregrtest/main.py
+++ b/Lib/test/libregrtest/main.py
@@ -384,15 +384,10 @@ class Regrtest:
 
             result = self.run_test(test_name, runtests, tracer)
 
-            # Unload the newly imported test modules (best effort
-            # finalization). To work around gh-115490, don't unload
-            # test.support.interpreters and its submodules even if they
-            # weren't loaded before.
-            keep = "test.support.interpreters"
+            # Unload the newly imported test modules (best effort finalization)
             new_modules = [module for module in sys.modules
                            if module not in save_modules and
-                                module.startswith(("test.", "test_"))
-                                and not module.startswith(keep)]
+                                module.startswith(("test.", "test_"))]
             for module in new_modules:
                 sys.modules.pop(module, None)
                 # Remove the attribute of the parent module.

diff --git a/Lib/test/test__xxinterpchannels.py b/Lib/test/test__xxinterpchannels.py
index cc2ed7849b0c0f869bb2a14ca44799c6015f4e7b..c5d29bd2dd911fc85d38514f7e6905c8b1568748 100644 (file)
--- a/Lib/test/test__xxinterpchannels.py
+++ b/Lib/test/test__xxinterpchannels.py
@@ -18,6 +18,11 @@ from test.test__xxsubinterpreters import (
 channels = import_helper.import_module('_xxinterpchannels')
 
 
+# Additional tests are found in Lib/test/test_interpreters/test_channels.py.
+# New tests should be added there.
+# XXX The tests here should be moved there.  See the note under LowLevelTests.
+
+
 ##################################
 # helpers
 

diff --git a/Lib/test/test_interpreters/test_channels.py b/Lib/test/test_interpreters/test_channels.py
index 07e503837bcf75b2d4f74f0ae2ea95223a536f94..57204e2776468d3234cf50a3b5be3ff89601d2a3 100644 (file)
--- a/Lib/test/test_interpreters/test_channels.py
+++ b/Lib/test/test_interpreters/test_channels.py
@@ -1,3 +1,4 @@
+import importlib
 import threading
 from textwrap import dedent
 import unittest
@@ -11,6 +12,24 @@ from test.support.interpreters import channels
 from .utils import _run_output, TestBase
 
 
+class LowLevelTests(TestBase):
+
+    # The behaviors in the low-level module is important in as much
+    # as it is exercised by the high-level module.  Therefore the
+    # most # important testing happens in the high-level tests.
+    # These low-level tests cover corner cases that are not
+    # encountered by the high-level module, thus they
+    # mostly shouldn't matter as much.
+
+    # Additional tests are found in Lib/test/test__xxinterpchannels.py.
+    # XXX Those should be either moved to LowLevelTests or eliminated
+    # in favor of high-level tests in this file.
+
+    def test_highlevel_reloaded(self):
+        # See gh-115490 (https://github.com/python/cpython/issues/115490).
+        importlib.reload(channels)
+
+
 class TestChannels(TestBase):
 
     def test_create(self):

diff --git a/Lib/test/test_interpreters/test_queues.py b/Lib/test/test_interpreters/test_queues.py
index 905ef035d43feb17846d876abd1e2449ac7447a3..0a1fdb41f7316687c09d5bef203a606da0c575a0 100644 (file)
--- a/Lib/test/test_interpreters/test_queues.py
+++ b/Lib/test/test_interpreters/test_queues.py
@@ -1,3 +1,4 @@
+import importlib
 import threading
 from textwrap import dedent
 import unittest
@@ -20,6 +21,20 @@ class TestBase(TestBase):
                 pass
 
 
+class LowLevelTests(TestBase):
+
+    # The behaviors in the low-level module is important in as much
+    # as it is exercised by the high-level module.  Therefore the
+    # most # important testing happens in the high-level tests.
+    # These low-level tests cover corner cases that are not
+    # encountered by the high-level module, thus they
+    # mostly shouldn't matter as much.
+
+    def test_highlevel_reloaded(self):
+        # See gh-115490 (https://github.com/python/cpython/issues/115490).
+        importlib.reload(queues)
+
+
 class QueueTests(TestBase):
 
     def test_create(self):

diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c
index c0d4fd080884343b2f73c138be18d5dd1c370eae..0ad184a78e8c1a9fd0487604af289cc4b0a1d76d 100644 (file)
--- a/Modules/_xxinterpchannelsmodule.c
+++ b/Modules/_xxinterpchannelsmodule.c
@@ -2675,12 +2675,17 @@ set_channelend_types(PyObject *mod, PyTypeObject *send, PyTypeObject *recv)
         return -1;
     }
 
-    if (state->send_channel_type != NULL
-        || state->recv_channel_type != NULL)
-    {
-        PyErr_SetString(PyExc_TypeError, "already registered");
-        return -1;
+    // Clear the old values if the .py module was reloaded.
+    if (state->send_channel_type != NULL) {
+        (void)_PyCrossInterpreterData_UnregisterClass(state->send_channel_type);
+        Py_CLEAR(state->send_channel_type);
     }
+    if (state->recv_channel_type != NULL) {
+        (void)_PyCrossInterpreterData_UnregisterClass(state->recv_channel_type);
+        Py_CLEAR(state->recv_channel_type);
+    }
+
+    // Add and register the types.
     state->send_channel_type = (PyTypeObject *)Py_NewRef(send);
     state->recv_channel_type = (PyTypeObject *)Py_NewRef(recv);
     if (ensure_xid_class(send, _channelend_shared) < 0) {

diff --git a/Modules/_xxinterpqueuesmodule.c b/Modules/_xxinterpqueuesmodule.c
index e35d1699cfea89d3d1b1f678b5f58dea029da3a3..1b76b6963ae0f13ae46d5e3252ca6c377f7decf9 100644 (file)
--- a/Modules/_xxinterpqueuesmodule.c
+++ b/Modules/_xxinterpqueuesmodule.c
@@ -169,6 +169,9 @@ static int
 clear_module_state(module_state *state)
 {
     /* external types */
+    if (state->queue_type != NULL) {
+        (void)_PyCrossInterpreterData_UnregisterClass(state->queue_type);
+    }
     Py_CLEAR(state->queue_type);
 
     /* QueueError */
@@ -1078,15 +1081,18 @@ set_external_queue_type(PyObject *module, PyTypeObject *queue_type)
 {
     module_state *state = get_module_state(module);
 
+    // Clear the old value if the .py module was reloaded.
     if (state->queue_type != NULL) {
-        PyErr_SetString(PyExc_TypeError, "already registered");
-        return -1;
+        (void)_PyCrossInterpreterData_UnregisterClass(
+                                state->queue_type);
+        Py_CLEAR(state->queue_type);
     }
-    state->queue_type = (PyTypeObject *)Py_NewRef(queue_type);
 
+    // Add and register the new type.
     if (ensure_xid_class(queue_type, _queueobj_shared) < 0) {
         return -1;
     }
+    state->queue_type = (PyTypeObject *)Py_NewRef(queue_type);
 
     return 0;
 }
@@ -1703,10 +1709,6 @@ module_clear(PyObject *mod)
 {
     module_state *state = get_module_state(mod);
 
-    if (state->queue_type != NULL) {
-        (void)_PyCrossInterpreterData_UnregisterClass(state->queue_type);
-    }
-
     // Now we clear the module state.
     clear_module_state(state);
     return 0;