From 5b68f4d9d7be36eec8e834d1889d1a7320d773fa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 22 Mar 2024 10:56:41 -0600 Subject: [PATCH 01/18] Identify unmanaged interpreters. --- Lib/test/support/interpreters/__init__.py | 50 +++++++----- Lib/test/test_interpreters/test_api.py | 97 ++++++++++++++++------- Lib/test/test_interpreters/utils.py | 3 +- Modules/_xxsubinterpretersmodule.c | 66 ++++++++++++--- 4 files changed, 158 insertions(+), 58 deletions(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index 60323c9874f9a0..cf47de8e11496f 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -74,51 +74,59 @@ def __str__(self): def create(): """Return a new (idle) Python interpreter.""" id = _interpreters.create(reqrefs=True) - return Interpreter(id) + return Interpreter(id, _owned=True) def list_all(): """Return all existing interpreters.""" - return [Interpreter(id) - for id, in _interpreters.list_all()] + return [Interpreter(id, _owned=owned) + for id, owned in _interpreters.list_all()] def get_current(): """Return the currently running interpreter.""" - id, = _interpreters.get_current() - return Interpreter(id) + id, owned = _interpreters.get_current() + return Interpreter(id, _owned=owned) def get_main(): """Return the main interpreter.""" - id, = _interpreters.get_main() - return Interpreter(id) + id, owned = _interpreters.get_main() + assert owned is False + return Interpreter(id, _owned=owned) _known = weakref.WeakValueDictionary() class Interpreter: - """A single Python interpreter.""" + """A single Python interpreter. - def __new__(cls, id, /): + Attributes: + + "id" - the unique process-global ID number for the interpreter + "owned" - indicates whether or not the interpreter was created + by interpreters.create() + """ + + def __new__(cls, id, /, _owned=None): # There is only one instance for any given ID. if not isinstance(id, int): raise TypeError(f'id must be an int, got {id!r}') id = int(id) + if _owned is None: + _owned = _interpreters.is_owned(id) try: self = _known[id] assert hasattr(self, '_ownsref') except KeyError: - # This may raise InterpreterNotFoundError: - _interpreters.incref(id) - try: - self = super().__new__(cls) - self._id = id - self._ownsref = True - except BaseException: - _interpreters.decref(id) - raise + self = super().__new__(cls) _known[id] = self + self._id = id + self._owned = _owned + self._ownsref = _owned + if _owned: + # This may raise InterpreterNotFoundError: + _interpreters.incref(id) return self def __repr__(self): @@ -143,7 +151,7 @@ def _decref(self): return self._ownsref = False try: - _interpreters.decref(self.id) + _interpreters.decref(self._id) except InterpreterNotFoundError: pass @@ -151,6 +159,10 @@ def _decref(self): def id(self): return self._id + @property + def owned(self): + return self._owned + def is_running(self): """Return whether or not the identified interpreter is running.""" return _interpreters.is_running(self._id) diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index abf66a7cde796c..aec74387167f75 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -172,10 +172,11 @@ def test_created_with_capi(self): text = self.run_temp_from_capi(f""" import {interpreters.__name__} as interpreters interp = interpreters.get_current() - print(interp.id) + print((interp.id, interp.owned)) """) - interpid = eval(text) + interpid, owned = eval(text) self.assertEqual(interpid, expected) + self.assertFalse(owned) class ListAllTests(TestBase): @@ -227,22 +228,22 @@ def test_created_with_capi(self): interpid4 = interpid3 + 1 interpid5 = interpid4 + 1 expected = [ - (mainid,), - (interpid1,), - (interpid2,), - (interpid3,), - (interpid4,), - (interpid5,), + (mainid, False), + (interpid1, True), + (interpid2, True), + (interpid3, True), + (interpid4, False), + (interpid5, True), ] expected2 = expected[:-2] text = self.run_temp_from_capi(f""" import {interpreters.__name__} as interpreters interp = interpreters.create() print( - [(i.id,) for i in interpreters.list_all()]) + [(i.id, i.owned) for i in interpreters.list_all()]) """) res = eval(text) - res2 = [(i.id,) for i in interpreters.list_all()] + res2 = [(i.id, i.owned) for i in interpreters.list_all()] self.assertEqual(res, expected) self.assertEqual(res2, expected2) @@ -298,6 +299,32 @@ def test_id_readonly(self): with self.assertRaises(AttributeError): interp.id = 1_000_000 + def test_owned(self): + main = interpreters.get_main() + interp = interpreters.create() + + with self.subTest('main'): + self.assertFalse(main.owned) + + with self.subTest('from _interpreters'): + self.assertTrue(interp.owned) + + with self.subTest('from C-API'): + text = self.run_temp_from_capi(f""" + import {interpreters.__name__} as interpreters + interp = interpreters.get_current() + print(interp.owned) + """) + owned = eval(text) + self.assertFalse(owned) + + with self.subTest('readonly'): + for value in (True, False): + with self.assertRaises(AttributeError): + interp.owned = value + with self.assertRaises(AttributeError): + main.owned = value + def test_hashable(self): interp = interpreters.create() expected = hash(interp.id) @@ -1115,7 +1142,7 @@ class LowLevelTests(TestBase): def interp_exists(self, interpid): try: - _interpreters.is_running(interpid) + _interpreters.is_owned(interpid) except InterpreterNotFoundError: return False else: @@ -1244,32 +1271,38 @@ def test_new_config(self): _interpreters.new_config(gil=value) def test_get_main(self): - interpid, = _interpreters.get_main() + interpid, owned = _interpreters.get_main() self.assertEqual(interpid, 0) + self.assertFalse(owned) + self.assertFalse( + _interpreters.is_owned(interpid)) def test_get_current(self): with self.subTest('main'): main, *_ = _interpreters.get_main() - interpid, = _interpreters.get_current() + interpid, owned = _interpreters.get_current() self.assertEqual(interpid, main) + self.assertFalse(owned) script = f""" import {_interpreters.__name__} as _interpreters - interpid, = _interpreters.get_current() - print(interpid) + interpid, owned = _interpreters.get_current() + print(interpid, owned) """ def parse_stdout(text): parts = text.split() - assert len(parts) == 1, parts - interpid, = parts + assert len(parts) == 2, parts + interpid, owned = parts interpid = int(interpid) - return interpid, + owned = eval(owned) + return interpid, owned with self.subTest('from _interpreters'): orig = _interpreters.create() text = self.run_and_capture(orig, script) - interpid, = parse_stdout(text) + interpid, owned = parse_stdout(text) self.assertEqual(interpid, orig) + self.assertTrue(owned) with self.subTest('from C-API'): last = 0 @@ -1277,8 +1310,9 @@ def parse_stdout(text): last = max(last, id) expected = last + 1 text = self.run_temp_from_capi(script) - interpid, = parse_stdout(text) + interpid, owned = parse_stdout(text) self.assertEqual(interpid, expected) + self.assertFalse(owned) def test_list_all(self): mainid, *_ = _interpreters.get_main() @@ -1286,17 +1320,17 @@ def test_list_all(self): interpid2 = _interpreters.create() interpid3 = _interpreters.create() expected = [ - (mainid,), - (interpid1,), - (interpid2,), - (interpid3,), + (mainid, False), + (interpid1, True), + (interpid2, True), + (interpid3, True), ] with self.subTest('main'): res = _interpreters.list_all() self.assertEqual(res, expected) - with self.subTest('from _interpreters'): + with self.subTest('via interp from _interpreters'): text = self.run_and_capture(interpid2, f""" import {_interpreters.__name__} as _interpreters print( @@ -1306,15 +1340,15 @@ def test_list_all(self): res = eval(text) self.assertEqual(res, expected) - with self.subTest('from C-API'): + with self.subTest('via interp from C-API'): interpid4 = interpid3 + 1 interpid5 = interpid4 + 1 expected2 = expected + [ - (interpid4,), - (interpid5,), + (interpid4, False), + (interpid5, True), ] expected3 = expected + [ - (interpid5,), + (interpid5, True), ] text = self.run_temp_from_capi(f""" import {_interpreters.__name__} as _interpreters @@ -1378,6 +1412,11 @@ def test_create(self): with self.assertRaises(ValueError): _interpreters.create(orig) + with self.subTest('is owned'): + interpid = _interpreters.create() + self.assertTrue( + _interpreters.is_owned(interpid)) + @requires_test_modules def test_destroy(self): with self.subTest('from _interpreters'): diff --git a/Lib/test/test_interpreters/utils.py b/Lib/test/test_interpreters/utils.py index d92179474959ef..6b0c78c5276753 100644 --- a/Lib/test/test_interpreters/utils.py +++ b/Lib/test/test_interpreters/utils.py @@ -545,7 +545,8 @@ def interpreter_from_capi(self, config=None, whence=None): @contextlib.contextmanager def interpreter_obj_from_capi(self, config='legacy'): with self.interpreter_from_capi(config) as interpid: - yield interpreters.Interpreter(interpid), interpid + interp = interpreters.Interpreter(interpid, _owned=False) + yield interp, interpid @contextlib.contextmanager def capturing(self, script): diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 37ac5a3f28aba9..80ca14b7ff9734 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -489,6 +489,24 @@ _run_in_interpreter(PyInterpreterState *interp, } +/* global state *************************************************************/ + +static void +set_owned(PyInterpreterState *interp) +{ +} + +static int +is_owned(PyInterpreterState *interp) +{ + // XXX + if (_Py_IsMainInterpreter(interp)) { + return 0; + } + return 1; +} + + /* module level code ********************************************************/ static PyObject * @@ -498,7 +516,8 @@ get_summary(PyInterpreterState *interp) if (idobj == NULL) { return NULL; } - PyObject *res = PyTuple_Pack(1, idobj); + PyObject *owned = is_owned(interp) ? Py_True : Py_False; + PyObject *res = PyTuple_Pack(2, idobj, owned); Py_DECREF(idobj); return res; } @@ -580,6 +599,8 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } + set_owned(interp); + if (reqrefs) { // Decref to 0 will destroy the interpreter. _PyInterpreterState_RequireIDRef(interp, 1); @@ -658,21 +679,19 @@ So does an unrecognized ID."); static PyObject * interp_list_all(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyObject *ids; - PyInterpreterState *interp; - - ids = PyList_New(0); + PyObject *ids = PyList_New(0); if (ids == NULL) { return NULL; } - interp = PyInterpreterState_Head(); + PyInterpreterState *interp = PyInterpreterState_Head(); while (interp != NULL) { PyObject *item = get_summary(interp); if (item == NULL) { Py_DECREF(ids); return NULL; } + // insert at front of list int res = PyList_Insert(ids, 0, item); Py_DECREF(item); @@ -688,7 +707,7 @@ interp_list_all(PyObject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(list_all_doc, -"list_all() -> [(ID,)]\n\ +"list_all() -> [(ID, owned)]\n\ \n\ Return a list containing the ID of every existing interpreter."); @@ -704,7 +723,7 @@ interp_get_current(PyObject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(get_current_doc, -"get_current() -> (ID,)\n\ +"get_current() -> (ID, owned)\n\ \n\ Return the ID of current interpreter."); @@ -717,10 +736,11 @@ interp_get_main(PyObject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(get_main_doc, -"get_main() -> (ID,)\n\ +"get_main() -> (ID, owned)\n\ \n\ Return the ID of main interpreter."); + static PyObject * interp_set___main___attrs(PyObject *self, PyObject *args) { @@ -775,6 +795,7 @@ PyDoc_STRVAR(set___main___attrs_doc, \n\ Bind the given attributes in the interpreter's __main__ module."); + static PyUnicodeObject * convert_script_arg(PyObject *arg, const char *fname, const char *displayname, const char *expected) @@ -1177,6 +1198,31 @@ PyDoc_STRVAR(whence_doc, Return an identifier for where the interpreter was created."); +static PyObject * +interp_is_owned(PyObject *self, PyObject *args, PyObject *kwds) +{ + static char *kwlist[] = {"id", NULL}; + PyObject *id; + if (!PyArg_ParseTupleAndKeywords(args, kwds, + "O:is_owned", kwlist, &id)) + { + return NULL; + } + + PyInterpreterState *interp = look_up_interp(id); + if (interp == NULL) { + return NULL; + } + + return is_owned(interp) ? Py_True : Py_False; +} + +PyDoc_STRVAR(is_owned_doc, +"is_owned(id) -> bool\n\ +\n\ +Return True if the interpreter was created by this module."); + + static PyObject * interp_incref(PyObject *self, PyObject *args, PyObject *kwds) { @@ -1308,6 +1354,8 @@ static PyMethodDef module_functions[] = { {"get_main", interp_get_main, METH_NOARGS, get_main_doc}, + {"is_owned", _PyCFunction_CAST(interp_is_owned), + METH_VARARGS | METH_KEYWORDS, is_owned_doc}, {"is_running", _PyCFunction_CAST(interp_is_running), METH_VARARGS | METH_KEYWORDS, is_running_doc}, {"get_config", _PyCFunction_CAST(interp_get_config), From 1811e50dcbad10ba30bbd7b20233db8051919d4a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 25 Mar 2024 17:09:57 -0600 Subject: [PATCH 02/18] Implement using a hashtable. --- Modules/_xxsubinterpretersmodule.c | 146 +++++++++++++++++++++++++++-- 1 file changed, 136 insertions(+), 10 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 80ca14b7ff9734..80cb4b3f66e6ed 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -489,21 +489,138 @@ _run_in_interpreter(PyInterpreterState *interp, } -/* global state *************************************************************/ +/* "owned" interpreters *****************************************************/ + +typedef _Py_hashtable_t owned_ids; + +static Py_uhash_t +hash_int64(const void *key) +{ +#if sizeof(int64_t) <= sizeof(Py_uhash_t) + return (Py_uhash_t)key; +#else + int64_t val = *(int64_t *)key; + return val % sizeof(Py_uhash_t); +#endif +} + +static int +compare_int64(const void *key1, const void *key2) +{ +#if sizeof(int64_t) <= sizeof(Py_uhash_t) + return (int64_t)key1 == (int64_t)key2; +#else + int64_t val1 = *(int64_t *)key1; + int64_t val2 = *(int64_t *)key2; + return val1 == val2; +#endif +} + +static int +init_owned(owned_ids *owned) +{ + _Py_hashtable_allocator_t alloc = (_Py_hashtable_allocator_t){ + .malloc = PyMem_RawMalloc, + .free = PyMem_RawFree, + }; + _globals.owned = _Py_hashtable_new_full( + hash_int64, compare_int64, NULL, NULL, &alloc); + if (_globals.owned == NULL) { + PyErr_NoMemory(); + return -1; + } + return 0; +} static void -set_owned(PyInterpreterState *interp) +clear_owned(owned_ids *owned) { + _Py_hashtable_destroy(owned); } static int -is_owned(PyInterpreterState *interp) +add_owned(owned_ids *owned, PyInterpreterState *interp) { - // XXX - if (_Py_IsMainInterpreter(interp)) { - return 0; + int64_t id = PyInterpreterState_GetID(interp); +#if sizeof(int64_t) <= sizeof(Py_uhash_t) + return _Py_hashtable_set(owned_ids, (const void *)id, 1); +#else + assert(0); +#endif +} + +static void +drop_owned(owned_ids *owned, PyInterpreterState *interp) +{ + int64_t id = PyInterpreterState_GetID(interp); +#if sizeof(int64_t) <= sizeof(Py_uhash_t) + _Py_hashtable_steal(owned_ids, (const void *)id); +#else + assert(0); +#endif +} + +static int +is_owned(owned_ids *owned, PyInterpreterState *interp) +{ + int64_t id = PyInterpreterState_GetID(interp); +#if sizeof(int64_t) <= sizeof(Py_uhash_t) + return _Py_hashtable_get_entry(owned_ids, (const void *)id) != NULL; +#else + assert(0); +#endif +} + + +/* global state *************************************************************/ + +/* globals is the process-global state for the module. It holds all + the data that we need to share between interpreters, so it cannot + hold PyObject values. */ +static struct globals { + PyMutex mutex; + int module_count; + _Py_hashtable_t *owned; +} _globals = {0}; + +static int +_globals_init(void) +{ + int res = -1; + PyMutex_Lock(&_globals.mutex); + + _globals.module_count++; + if (_globals.module_count > 1) { + // Already initialized. + res = 0; + goto finally; + } + + if (init_owned(&_globals.owned) < 0) { + goto finally; } - return 1; + + res = 0; + +finally: + PyMutex_Unlock(&_globals.mutex); + return res; +} + +static void +_globals_fini(void) +{ + PyMutex_Lock(&_globals.mutex); + + _globals.module_count--; + if (_globals.module_count > 0) { + goto finally; + } + + clear_owned(&_globals.owned); + +finally: + PyMutex_Unlock(&_globals.mutex); } @@ -516,7 +633,7 @@ get_summary(PyInterpreterState *interp) if (idobj == NULL) { return NULL; } - PyObject *owned = is_owned(interp) ? Py_True : Py_False; + PyObject *owned = is_owned(&_globals.owned, interp) ? Py_True : Py_False; PyObject *res = PyTuple_Pack(2, idobj, owned); Py_DECREF(idobj); return res; @@ -599,7 +716,7 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - set_owned(interp); + add_owned(&_globals.owned, interp); if (reqrefs) { // Decref to 0 will destroy the interpreter. @@ -661,6 +778,8 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } + drop_owned(&_globals.owned, interp); + // Destroy the interpreter. _PyXI_EndInterpreter(interp, NULL, NULL); @@ -1214,7 +1333,7 @@ interp_is_owned(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - return is_owned(interp) ? Py_True : Py_False; + return is_owned(&_globals.owned, interp) ? Py_True : Py_False; } PyDoc_STRVAR(is_owned_doc, @@ -1401,6 +1520,10 @@ module_exec(PyObject *mod) PyInterpreterState *interp = PyInterpreterState_Get(); module_state *state = get_module_state(mod); + if (_globals_init() != 0) { + return -1; + } + #define ADD_WHENCE(NAME) \ if (PyModule_AddIntConstant(mod, "WHENCE_" #NAME, \ _PyInterpreterState_WHENCE_##NAME) < 0) \ @@ -1434,6 +1557,7 @@ module_exec(PyObject *mod) return 0; error: + _globals_fini(); return -1; } @@ -1467,6 +1591,8 @@ module_free(void *mod) module_state *state = get_module_state(mod); assert(state != NULL); clear_module_state(state); + + _globals_fini(); } static struct PyModuleDef moduledef = { From 05a39235771d62f656824d4c2a059993ffde9747 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 1 Apr 2024 18:33:27 -0600 Subject: [PATCH 03/18] Call drop_owned() after Py_EndInterpreter(), not before. --- Modules/_xxsubinterpretersmodule.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 80cb4b3f66e6ed..b20861c89cd22a 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -550,9 +550,8 @@ add_owned(owned_ids *owned, PyInterpreterState *interp) } static void -drop_owned(owned_ids *owned, PyInterpreterState *interp) +drop_owned(owned_ids *owned, int64_t interpid) { - int64_t id = PyInterpreterState_GetID(interp); #if sizeof(int64_t) <= sizeof(Py_uhash_t) _Py_hashtable_steal(owned_ids, (const void *)id); #else @@ -758,6 +757,7 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) if (interp == NULL) { return NULL; } + int64_t interpid = PyInterpreterState_GetID(interp); // Ensure we don't try to destroy the current interpreter. PyInterpreterState *current = _get_current_interp(); @@ -778,11 +778,11 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - drop_owned(&_globals.owned, interp); - // Destroy the interpreter. _PyXI_EndInterpreter(interp, NULL, NULL); + drop_owned(&_globals.owned, interpid); + Py_RETURN_NONE; } From 6f7c83d8f6ab9edbe5dd46f387b1089aac2d9235 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 25 Mar 2024 17:32:17 -0600 Subject: [PATCH 04/18] Use a linked list. --- Modules/_xxsubinterpretersmodule.c | 137 ++++++++++++++++++----------- 1 file changed, 85 insertions(+), 52 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index b20861c89cd22a..446bde611295ee 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -491,83 +491,116 @@ _run_in_interpreter(PyInterpreterState *interp, /* "owned" interpreters *****************************************************/ -typedef _Py_hashtable_t owned_ids; - -static Py_uhash_t -hash_int64(const void *key) -{ -#if sizeof(int64_t) <= sizeof(Py_uhash_t) - return (Py_uhash_t)key; -#else - int64_t val = *(int64_t *)key; - return val % sizeof(Py_uhash_t); -#endif -} +struct owned_id { + int64_t id; + struct owned_id *next; +}; -static int -compare_int64(const void *key1, const void *key2) -{ -#if sizeof(int64_t) <= sizeof(Py_uhash_t) - return (int64_t)key1 == (int64_t)key2; -#else - int64_t val1 = *(int64_t *)key1; - int64_t val2 = *(int64_t *)key2; - return val1 == val2; -#endif -} +typedef struct { + PyMutex mutex; + struct owned_id *head; + Py_ssize_t count; +} owned_ids; static int init_owned(owned_ids *owned) { - _Py_hashtable_allocator_t alloc = (_Py_hashtable_allocator_t){ - .malloc = PyMem_RawMalloc, - .free = PyMem_RawFree, - }; - _globals.owned = _Py_hashtable_new_full( - hash_int64, compare_int64, NULL, NULL, &alloc); - if (_globals.owned == NULL) { - PyErr_NoMemory(); - return -1; - } return 0; } static void clear_owned(owned_ids *owned) { - _Py_hashtable_destroy(owned); + PyMutex_Lock(&owned->mutex); + struct owned_id *cur = owned->head; + Py_ssize_t count = 0; + while (cur != NULL) { + struct owned_id *next = cur->next; + PyMem_RawFree(cur); + cur = next; + count += 1; + } + assert(count == owned->count); + owned->head = NULL; + owned->count = 0; + PyMutex_Unlock(&owned->mutex); } -static int +static struct owned_id * +_find_owned(owned_ids *owned, int64_t interpid, struct owned_id **p_prev) +{ + // The caller must manage the lock. + if (owned->head == NULL) { + return NULL; + } + + struct owned_id *prev = NULL; + struct owned_id *cur = owned->head; + while (cur != NULL) { + if (cur->id == interpid) { + break; + } + prev = cur; + cur = cur->next; + } + if (cur == NULL) { + return NULL; + } + + if (p_prev != NULL) { + *p_prev = prev; + } + return cur; +} + +static void add_owned(owned_ids *owned, PyInterpreterState *interp) { + PyMutex_Lock(&owned->mutex); int64_t id = PyInterpreterState_GetID(interp); -#if sizeof(int64_t) <= sizeof(Py_uhash_t) - return _Py_hashtable_set(owned_ids, (const void *)id, 1); -#else - assert(0); -#endif + struct owned_id *new = PyMem_RawMalloc(sizeof(struct owned_id)); + assert(new != NULL); + new->id = id; + new->next = owned->head; + owned->head = new; + owned->count += 1; + PyMutex_Unlock(&owned->mutex); } static void drop_owned(owned_ids *owned, int64_t interpid) { -#if sizeof(int64_t) <= sizeof(Py_uhash_t) - _Py_hashtable_steal(owned_ids, (const void *)id); -#else - assert(0); -#endif + PyMutex_Lock(&owned->mutex); + + struct owned_id *prev; + struct owned_id *found = _find_owned(owned, interpid, &prev); + if (found != NULL) { + if (prev == NULL) { + assert(found == owned->head); + owned->head = found->next; + } + else { + assert(found != owned->head); + prev->next = found->next; + } + PyMem_RawFree(found); + owned->count -= 1; + } + + PyMutex_Unlock(&owned->mutex); } static int is_owned(owned_ids *owned, PyInterpreterState *interp) { - int64_t id = PyInterpreterState_GetID(interp); -#if sizeof(int64_t) <= sizeof(Py_uhash_t) - return _Py_hashtable_get_entry(owned_ids, (const void *)id) != NULL; -#else - assert(0); -#endif + int64_t interpid = PyInterpreterState_GetID(interp); + PyMutex_Lock(&owned->mutex); + + struct owned_id *found = _find_owned(owned, interpid, NULL); + int res = found != NULL; + + PyMutex_Unlock(&owned->mutex); + return res; } @@ -579,7 +612,7 @@ is_owned(owned_ids *owned, PyInterpreterState *interp) static struct globals { PyMutex mutex; int module_count; - _Py_hashtable_t *owned; + owned_ids owned; } _globals = {0}; static int From 3a60142d907850beda1d32f9a8c3b0befc9eb51c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 25 Mar 2024 17:34:30 -0600 Subject: [PATCH 05/18] Add restrictions. --- Lib/test/support/interpreters/__init__.py | 18 +- Lib/test/test_interpreters/test_api.py | 115 +++++++------ Lib/test/test_interpreters/utils.py | 8 + Modules/_xxsubinterpretersmodule.c | 193 +++++++++++++++------- 4 files changed, 214 insertions(+), 120 deletions(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index cf47de8e11496f..38f7650ff22831 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -106,6 +106,10 @@ class Interpreter: "id" - the unique process-global ID number for the interpreter "owned" - indicates whether or not the interpreter was created by interpreters.create() + + If interp.owned is false then any method that modifies the + interpreter will fail, i.e. .close(), .prepare_main(), .exec(), + and .call() """ def __new__(cls, id, /, _owned=None): @@ -165,7 +169,11 @@ def owned(self): def is_running(self): """Return whether or not the identified interpreter is running.""" - return _interpreters.is_running(self._id) + # require_owned is okay since this doesn't modify the interpreter. + return _interpreters.is_running(self._id, require_owned=False) + + # Everything past here is available only to interpreters created by + # interpreters.create(). def close(self): """Finalize and destroy the interpreter. @@ -173,7 +181,7 @@ def close(self): Attempting to destroy the current interpreter results in an InterpreterError. """ - return _interpreters.destroy(self._id) + return _interpreters.destroy(self._id, require_owned=True) def prepare_main(self, ns=None, /, **kwargs): """Bind the given values into the interpreter's __main__. @@ -181,7 +189,7 @@ def prepare_main(self, ns=None, /, **kwargs): The values must be shareable. """ ns = dict(ns, **kwargs) if ns is not None else kwargs - _interpreters.set___main___attrs(self._id, ns) + _interpreters.set___main___attrs(self._id, ns, require_owned=True) def exec(self, code, /): """Run the given source code in the interpreter. @@ -201,7 +209,7 @@ def exec(self, code, /): that time, the previous interpreter is allowed to run in other threads. """ - excinfo = _interpreters.exec(self._id, code) + excinfo = _interpreters.exec(self._id, code, require_owned=True) if excinfo is not None: raise ExecutionFailed(excinfo) @@ -221,7 +229,7 @@ def call(self, callable, /): # XXX Support args and kwargs. # XXX Support arbitrary callables. # XXX Support returning the return value (e.g. via pickle). - excinfo = _interpreters.call(self._id, callable) + excinfo = _interpreters.call(self._id, callable, require_owned=True) if excinfo is not None: raise ExecutionFailed(excinfo) diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index aec74387167f75..2b6682e29bad21 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -594,41 +594,42 @@ def test_created_with_capi(self): with self.subTest('running __main__ (from self)'): with self.interpreter_from_capi() as interpid: with self.assertRaisesRegex(ExecutionFailed, - 'InterpreterError.*current'): + 'InterpreterError.*unrecognized'): self.run_from_capi(interpid, script, main=True) with self.subTest('running, but not __main__ (from self)'): with self.assertRaisesRegex(ExecutionFailed, - 'InterpreterError.*current'): + 'InterpreterError.*unrecognized'): self.run_temp_from_capi(script) with self.subTest('running __main__ (from other)'): with self.interpreter_obj_from_capi() as (interp, interpid): with self.running_from_capi(interpid, main=True): - with self.assertRaisesRegex(InterpreterError, 'running'): + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): interp.close() # Make sure it wssn't closed. self.assertTrue( - interp.is_running()) + self.interp_exists(interpid)) - # The rest must be skipped until we deal with running threads when - # interp.close() is called. - return + # The rest would be skipped until we deal with running threads when + # interp.close() is called. However, the "owned" restrictions + # trigger first. with self.subTest('running, but not __main__ (from other)'): with self.interpreter_obj_from_capi() as (interp, interpid): with self.running_from_capi(interpid, main=False): - with self.assertRaisesRegex(InterpreterError, 'not managed'): + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): interp.close() # Make sure it wssn't closed. - self.assertFalse(interp.is_running()) + self.assertTrue( + self.interp_exists(interpid)) with self.subTest('not running (from other)'): - with self.interpreter_obj_from_capi() as (interp, _): - with self.assertRaisesRegex(InterpreterError, 'not managed'): + with self.interpreter_obj_from_capi() as (interp, interpid): + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): interp.close() - # Make sure it wssn't closed. - self.assertFalse(interp.is_running()) + self.assertTrue( + self.interp_exists(interpid)) class TestInterpreterPrepareMain(TestBase): @@ -697,12 +698,11 @@ def test_running(self): @requires_test_modules def test_created_with_capi(self): - with self.interpreter_from_capi() as interpid: - interp = interpreters.Interpreter(interpid) - interp.prepare_main({'spam': True}) - rc = _testinternalcapi.exec_interpreter(interpid, - 'assert spam is True') - assert rc == 0, rc + with self.interpreter_obj_from_capi() as (interp, interpid): + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): + interp.prepare_main({'spam': True}) + with self.assertRaisesRegex(ExecutionFailed, 'NameError'): + self.run_from_capi(interpid, 'assert spam is True') class TestInterpreterExec(TestBase): @@ -861,7 +861,7 @@ def task(): def test_created_with_capi(self): with self.interpreter_obj_from_capi() as (interp, _): - with self.assertRaisesRegex(ExecutionFailed, 'it worked'): + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): interp.exec('raise Exception("it worked!")') # test_xxsubinterpreters covers the remaining @@ -1140,14 +1140,6 @@ class LowLevelTests(TestBase): # encountered by the high-level module, thus they # mostly shouldn't matter as much. - def interp_exists(self, interpid): - try: - _interpreters.is_owned(interpid) - except InterpreterNotFoundError: - return False - else: - return True - def test_new_config(self): # This test overlaps with # test.test_capi.test_misc.InterpreterConfigTests. @@ -1438,7 +1430,11 @@ def test_destroy(self): with self.subTest('from C-API'): interpid = _testinternalcapi.create_interpreter() - _interpreters.destroy(interpid) + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): + _interpreters.destroy(interpid) + self.assertTrue( + self.interp_exists(interpid)) + _interpreters.destroy(interpid, require_owned=False) self.assertFalse( self.interp_exists(interpid)) @@ -1450,14 +1446,7 @@ def test_get_config(self): expected = _interpreters.new_config('legacy') expected.gil = 'own' interpid, *_ = _interpreters.get_main() - config = _interpreters.get_config(interpid) - self.assert_ns_equal(config, expected) - - with self.subTest('main'): - expected = _interpreters.new_config('legacy') - expected.gil = 'own' - interpid, *_ = _interpreters.get_main() - config = _interpreters.get_config(interpid) + config = _interpreters.get_config(interpid, require_owned=False) self.assert_ns_equal(config, expected) with self.subTest('isolated'): @@ -1475,7 +1464,9 @@ def test_get_config(self): with self.subTest('from C-API'): orig = _interpreters.new_config('isolated') with self.interpreter_from_capi(orig) as interpid: - config = _interpreters.get_config(interpid) + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): + _interpreters.get_config(interpid) + config = _interpreters.get_config(interpid, require_owned=False) self.assert_ns_equal(config, orig) @requires_test_modules @@ -1523,38 +1514,40 @@ def test_whence(self): self.assertEqual(whence, _interpreters.WHENCE_LEGACY_CAPI) def test_is_running(self): - with self.subTest('main'): - interpid, *_ = _interpreters.get_main() - running = _interpreters.is_running(interpid) - self.assertTrue(running) + def check(interpid, expected): + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): + _interpreters.is_running(interpid) + running = _interpreters.is_running(interpid, require_owned=False) + self.assertIs(running, expected) with self.subTest('from _interpreters (running)'): interpid = _interpreters.create() with self.running(interpid): running = _interpreters.is_running(interpid) - self.assertTrue(running) + self.assertTrue(running) with self.subTest('from _interpreters (not running)'): interpid = _interpreters.create() running = _interpreters.is_running(interpid) self.assertFalse(running) + with self.subTest('main'): + interpid, *_ = _interpreters.get_main() + check(interpid, True) + with self.subTest('from C-API (running __main__)'): with self.interpreter_from_capi() as interpid: with self.running_from_capi(interpid, main=True): - running = _interpreters.is_running(interpid) - self.assertTrue(running) + check(interpid, True) with self.subTest('from C-API (running, but not __main__)'): with self.interpreter_from_capi() as interpid: with self.running_from_capi(interpid, main=False): - running = _interpreters.is_running(interpid) - self.assertFalse(running) + check(interpid, False) with self.subTest('from C-API (not running)'): with self.interpreter_from_capi() as interpid: - running = _interpreters.is_running(interpid) - self.assertFalse(running) + check(interpid, False) def test_exec(self): with self.subTest('run script'): @@ -1591,7 +1584,13 @@ def test_exec(self): with self.subTest('from C-API'): with self.interpreter_from_capi() as interpid: - exc = _interpreters.exec(interpid, 'raise Exception("it worked!")') + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): + _interpreters.exec(interpid, 'raise Exception("it worked!")') + exc = _interpreters.exec( + interpid, + 'raise Exception("it worked!")', + require_owned=False, + ) self.assertIsNot(exc, None) self.assertEqual(exc.msg, 'it worked!') @@ -1616,6 +1615,7 @@ def test_call(self): errdisplay=exc.errdisplay, )) + @requires_test_modules def test_set___main___attrs(self): with self.subTest('from _interpreters'): interpid = _interpreters.create() @@ -1637,9 +1637,18 @@ def test_set___main___attrs(self): with self.subTest('from C-API'): with self.interpreter_from_capi() as interpid: - _interpreters.set___main___attrs(interpid, {'spam': True}) - exc = _interpreters.exec(interpid, 'assert spam is True') - self.assertIsNone(exc) + with self.assertRaisesRegex(InterpreterError, 'unrecognized'): + _interpreters.set___main___attrs(interpid, {'spam': True}) + _interpreters.set___main___attrs( + interpid, + {'spam': True}, + require_owned=False, + ) + rc = _testinternalcapi.exec_interpreter( + interpid, + 'assert spam is True', + ) + self.assertEqual(rc, 0) if __name__ == '__main__': diff --git a/Lib/test/test_interpreters/utils.py b/Lib/test/test_interpreters/utils.py index 6b0c78c5276753..14ea43d4710c79 100644 --- a/Lib/test/test_interpreters/utils.py +++ b/Lib/test/test_interpreters/utils.py @@ -509,6 +509,14 @@ def run_and_capture(self, interp, script): else: return text + def interp_exists(self, interpid): + try: + _interpreters.is_owned(interpid) + except _interpreters.InterpreterNotFoundError: + return False + else: + return True + @requires_test_modules @contextlib.contextmanager def interpreter_from_capi(self, config=None, whence=None): diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 446bde611295ee..608e05a25e60c1 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -658,6 +658,36 @@ _globals_fini(void) /* module level code ********************************************************/ +static PyInterpreterState * +resolve_interp(PyObject *idobj, int reqowned, const char *op) +{ + PyInterpreterState *interp; + if (idobj == NULL) { + interp = PyInterpreterState_Get(); + } + else { + interp = look_up_interp(idobj); + if (interp == NULL) { + return NULL; + } + } + + if (reqowned && !is_owned(&_globals.owned, interp)) { + if (idobj == NULL) { + PyErr_Format(PyExc_InterpreterError, + "cannot %s unrecognized current interpreter", op); + } + else { + PyErr_Format(PyExc_InterpreterError, + "cannot %s unrecognized interpreter %R", op, idobj); + } + return NULL; + } + + return interp; +} + + static PyObject * get_summary(PyInterpreterState *interp) { @@ -777,16 +807,18 @@ is \"isolated\"."); static PyObject * interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", NULL}; + static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *id; + int reqowned = 1; // XXX Use "L" for id? if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O:destroy", kwlist, &id)) { + "O|$p:destroy", kwlist, &id, &reqowned)) + { return NULL; } // Look up the interpreter. - PyInterpreterState *interp = look_up_interp(id); + PyInterpreterState *interp = resolve_interp(id, reqowned, "destroy"); if (interp == NULL) { return NULL; } @@ -820,7 +852,7 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(destroy_doc, -"destroy(id)\n\ +"destroy(id, *, require_owned=True)\n\ \n\ Destroy the identified interpreter.\n\ \n\ @@ -894,17 +926,21 @@ Return the ID of main interpreter."); static PyObject * -interp_set___main___attrs(PyObject *self, PyObject *args) +interp_set___main___attrs(PyObject *self, PyObject *args, PyObject *kwargs) { + static char *kwlist[] = {"id", "updates", "require_owned", NULL}; PyObject *id, *updates; - if (!PyArg_ParseTuple(args, "OO:" MODULE_NAME_STR ".set___main___attrs", - &id, &updates)) + int reqowned = 1; + if (!PyArg_ParseTupleAndKeywords(args, kwargs, + "OO|$p:" MODULE_NAME_STR ".set___main___attrs", + kwlist, &id, &updates, &reqowned)) { return NULL; } // Look up the interpreter. - PyInterpreterState *interp = look_up_interp(id); + PyInterpreterState *interp = resolve_interp(id, reqowned, + "update __main__ for"); if (interp == NULL) { return NULL; } @@ -943,7 +979,7 @@ interp_set___main___attrs(PyObject *self, PyObject *args) } PyDoc_STRVAR(set___main___attrs_doc, -"set___main___attrs(id, ns)\n\ +"set___main___attrs(id, ns, *, require_owned=True)\n\ \n\ Bind the given attributes in the interpreter's __main__ module."); @@ -1025,16 +1061,9 @@ convert_code_arg(PyObject *arg, const char *fname, const char *displayname, } static int -_interp_exec(PyObject *self, - PyObject *id_arg, PyObject *code_arg, PyObject *shared_arg, - PyObject **p_excinfo) +_interp_exec(PyObject *self, PyInterpreterState *interp, + PyObject *code_arg, PyObject *shared_arg, PyObject **p_excinfo) { - // Look up the interpreter. - PyInterpreterState *interp = look_up_interp(id_arg); - if (interp == NULL) { - return -1; - } - // Extract code. Py_ssize_t codestrlen = -1; PyObject *bytes_obj = NULL; @@ -1059,12 +1088,19 @@ _interp_exec(PyObject *self, static PyObject * interp_exec(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "code", "shared", NULL}; + static char *kwlist[] = {"id", "code", "shared", "require_owned", NULL}; PyObject *id, *code; PyObject *shared = NULL; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "OO|O:" MODULE_NAME_STR ".exec", kwlist, - &id, &code, &shared)) { + "OO|O$p:" MODULE_NAME_STR ".exec", kwlist, + &id, &code, &shared, &reqowned)) + { + return NULL; + } + + PyInterpreterState *interp = resolve_interp(id, reqowned, "exec code for"); + if (interp == NULL) { return NULL; } @@ -1082,7 +1118,7 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) } PyObject *excinfo = NULL; - int res = _interp_exec(self, id, code, shared, &excinfo); + int res = _interp_exec(self, interp, code, shared, &excinfo); Py_DECREF(code); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); @@ -1092,7 +1128,7 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(exec_doc, -"exec(id, code, shared=None)\n\ +"exec(id, code, shared=None, *, require_owned=True)\n\ \n\ Execute the provided code in the identified interpreter.\n\ This is equivalent to running the builtin exec() under the target\n\ @@ -1111,13 +1147,22 @@ is ignored, including its __globals__ dict."); static PyObject * interp_call(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "callable", "args", "kwargs", NULL}; + static char *kwlist[] = {"id", "callable", "args", "kwargs", + "require_owned", NULL}; PyObject *id, *callable; PyObject *args_obj = NULL; PyObject *kwargs_obj = NULL; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "OO|OO:" MODULE_NAME_STR ".call", kwlist, - &id, &callable, &args_obj, &kwargs_obj)) { + "OO|OO$p:" MODULE_NAME_STR ".call", kwlist, + &id, &callable, &args_obj, &kwargs_obj, + &reqowned)) + { + return NULL; + } + + PyInterpreterState *interp = resolve_interp(id, reqowned, "make a call in"); + if (interp == NULL) { return NULL; } @@ -1137,7 +1182,7 @@ interp_call(PyObject *self, PyObject *args, PyObject *kwds) } PyObject *excinfo = NULL; - int res = _interp_exec(self, id, code, NULL, &excinfo); + int res = _interp_exec(self, interp, code, NULL, &excinfo); Py_DECREF(code); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); @@ -1147,7 +1192,7 @@ interp_call(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(call_doc, -"call(id, callable, args=None, kwargs=None)\n\ +"call(id, callable, args=None, kwargs=None, *, require_owned=True)\n\ \n\ Call the provided object in the identified interpreter.\n\ Pass the given args and kwargs, if possible.\n\ @@ -1161,12 +1206,19 @@ is ignored, including its __globals__ dict."); static PyObject * interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "script", "shared", NULL}; + static char *kwlist[] = {"id", "script", "shared", "require_owned", NULL}; PyObject *id, *script; PyObject *shared = NULL; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "OU|O:" MODULE_NAME_STR ".run_string", kwlist, - &id, &script, &shared)) { + "OU|O$p:" MODULE_NAME_STR ".run_string", + kwlist, &id, &script, &shared, &reqowned)) + { + return NULL; + } + + PyInterpreterState *interp = resolve_interp(id, reqowned, "run a string in"); + if (interp == NULL) { return NULL; } @@ -1177,7 +1229,7 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) } PyObject *excinfo = NULL; - int res = _interp_exec(self, id, script, shared, &excinfo); + int res = _interp_exec(self, interp, script, shared, &excinfo); Py_DECREF(script); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); @@ -1187,7 +1239,7 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(run_string_doc, -"run_string(id, script, shared=None)\n\ +"run_string(id, script, shared=None, *, require_owned=True)\n\ \n\ Execute the provided string in the identified interpreter.\n\ \n\ @@ -1196,12 +1248,20 @@ Execute the provided string in the identified interpreter.\n\ static PyObject * interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "func", "shared", NULL}; + static char *kwlist[] = {"id", "func", "shared", "require_owned", NULL}; PyObject *id, *func; PyObject *shared = NULL; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "OO|O:" MODULE_NAME_STR ".run_func", kwlist, - &id, &func, &shared)) { + "OO|O$p:" MODULE_NAME_STR ".run_func", + kwlist, &id, &func, &shared, &reqowned)) + { + return NULL; + } + + PyInterpreterState *interp = resolve_interp(id, reqowned, + "run a function in"); + if (interp == NULL) { return NULL; } @@ -1213,7 +1273,7 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) } PyObject *excinfo = NULL; - int res = _interp_exec(self, id, (PyObject *)code, shared, &excinfo); + int res = _interp_exec(self, interp, (PyObject *)code, shared, &excinfo); Py_DECREF(code); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); @@ -1223,7 +1283,7 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(run_func_doc, -"run_func(id, func, shared=None)\n\ +"run_func(id, func, shared=None, *, require_owned=True)\n\ \n\ Execute the body of the provided function in the identified interpreter.\n\ Code objects are also supported. In both cases, closures and args\n\ @@ -1259,17 +1319,22 @@ False otherwise."); static PyObject * interp_is_running(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", NULL}; + static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *id; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O:is_running", kwlist, &id)) { + "O|$p:is_running", kwlist, + &id, &reqowned)) + { return NULL; } - PyInterpreterState *interp = look_up_interp(id); + PyInterpreterState *interp = resolve_interp(id, reqowned, + "check if running for"); if (interp == NULL) { return NULL; } + if (is_running_main(interp)) { Py_RETURN_TRUE; } @@ -1277,7 +1342,7 @@ interp_is_running(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(is_running_doc, -"is_running(id) -> bool\n\ +"is_running(id, *, require_owned=True) -> bool\n\ \n\ Return whether or not the identified interpreter is running."); @@ -1285,23 +1350,23 @@ Return whether or not the identified interpreter is running."); static PyObject * interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", NULL}; + static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *idobj = NULL; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O:get_config", kwlist, &idobj)) + "O|$p:get_config", kwlist, + &idobj, &reqowned)) { return NULL; } - - PyInterpreterState *interp; - if (idobj == NULL) { - interp = PyInterpreterState_Get(); + if (idobj == Py_None) { + idobj = NULL; } - else { - interp = _PyInterpreterState_LookUpIDObject(idobj); - if (interp == NULL) { - return NULL; - } + + PyInterpreterState *interp = resolve_interp(idobj, reqowned, + "get the config of"); + if (interp == NULL) { + return NULL; } PyInterpreterConfig config; @@ -1319,7 +1384,7 @@ interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(get_config_doc, -"get_config(id) -> types.SimpleNamespace\n\ +"get_config(id, *, require_owned=True) -> types.SimpleNamespace\n\ \n\ Return a representation of the config used to initialize the interpreter."); @@ -1378,17 +1443,18 @@ Return True if the interpreter was created by this module."); static PyObject * interp_incref(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "implieslink", NULL}; + static char *kwlist[] = {"id", "implieslink", "require_owned", NULL}; PyObject *id; int implieslink = 0; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O|$p:incref", kwlist, - &id, &implieslink)) + "O|$pp:incref", kwlist, + &id, &implieslink, &reqowned)) { return NULL; } - PyInterpreterState *interp = look_up_interp(id); + PyInterpreterState *interp = resolve_interp(id, reqowned, "incref"); if (interp == NULL) { return NULL; } @@ -1406,17 +1472,20 @@ interp_incref(PyObject *self, PyObject *args, PyObject *kwds) static PyObject * interp_decref(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", NULL}; + static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *id; + int reqowned = 1; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O:decref", kwlist, &id)) { + "O|$p:decref", kwlist, &id, &reqowned)) + { return NULL; } - PyInterpreterState *interp = look_up_interp(id); + PyInterpreterState *interp = resolve_interp(id, reqowned, "decref"); if (interp == NULL) { return NULL; } + _PyInterpreterState_IDDecref(interp); Py_RETURN_NONE; @@ -1524,7 +1593,7 @@ static PyMethodDef module_functions[] = { METH_VARARGS | METH_KEYWORDS, run_func_doc}, {"set___main___attrs", _PyCFunction_CAST(interp_set___main___attrs), - METH_VARARGS, set___main___attrs_doc}, + METH_VARARGS | METH_KEYWORDS, set___main___attrs_doc}, {"incref", _PyCFunction_CAST(interp_incref), METH_VARARGS | METH_KEYWORDS, NULL}, From 98fc876ba91f69667c89760693c4abc21f1d144e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 25 Mar 2024 14:40:59 -0600 Subject: [PATCH 06/18] Hide unmanaged interpreters. --- Lib/test/support/interpreters/__init__.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index 38f7650ff22831..5eefae2d7de5ae 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -79,13 +79,18 @@ def create(): def list_all(): """Return all existing interpreters.""" + mainid = _interpreters.get_main() return [Interpreter(id, _owned=owned) - for id, owned in _interpreters.list_all()] + for id, owned in _interpreters.list_all() + if owned or id == mainid] def get_current(): """Return the currently running interpreter.""" id, owned = _interpreters.get_current() + if not owned and id != _interpreters.get_main(): + # XXX Support this? + raise InterpreterError('current interpreter was created externally') return Interpreter(id, _owned=owned) From c2629b78c4e8f393721f68f008a119431efb55b3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 25 Mar 2024 14:20:05 -0600 Subject: [PATCH 07/18] Use Interpreter for unmanaged interpreters. --- Lib/test/support/interpreters/__init__.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index 5eefae2d7de5ae..9f68f157712a0d 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -79,18 +79,13 @@ def create(): def list_all(): """Return all existing interpreters.""" - mainid = _interpreters.get_main() return [Interpreter(id, _owned=owned) - for id, owned in _interpreters.list_all() - if owned or id == mainid] + for id, owned in _interpreters.list_all()] def get_current(): """Return the currently running interpreter.""" id, owned = _interpreters.get_current() - if not owned and id != _interpreters.get_main(): - # XXX Support this? - raise InterpreterError('current interpreter was created externally') return Interpreter(id, _owned=owned) @@ -168,6 +163,7 @@ def _decref(self): def id(self): return self._id + # XXX Is this the right name? @property def owned(self): return self._owned From f1e2461cb84ffca9902f598129be73b460c32fde Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 Apr 2024 16:28:32 -0600 Subject: [PATCH 08/18] Fix other tests. --- Lib/test/test__xxsubinterpreters.py | 2 +- Lib/test/test_capi/test_misc.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test__xxsubinterpreters.py b/Lib/test/test__xxsubinterpreters.py index c8c964f642f1cf..6251afcb606fea 100644 --- a/Lib/test/test__xxsubinterpreters.py +++ b/Lib/test/test__xxsubinterpreters.py @@ -309,7 +309,7 @@ class IsRunningTests(TestBase): def test_main(self): main, *_ = _interpreters.get_main() - self.assertTrue(_interpreters.is_running(main)) + self.assertTrue(_interpreters.is_running(main, require_owned=False)) @unittest.skip('Fails on FreeBSD') def test_subinterpreter(self): diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 35d6a209122a99..1e0995be09774d 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2428,7 +2428,7 @@ def new_interp(config): expected = _interpreters.new_config('legacy') expected.gil = 'own' interpid, *_ = _interpreters.get_main() - config = _interpreters.get_config(interpid) + config = _interpreters.get_config(interpid, require_owned=False) self.assert_ns_equal(config, expected) with self.subTest('isolated'): From 55a1c3100ea2be8b8fbe53dbf3b3acab2c1faf92 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 Apr 2024 16:52:52 -0600 Subject: [PATCH 09/18] Default to require_owned=False. --- Lib/test/test__xxsubinterpreters.py | 2 +- Lib/test/test_capi/test_misc.py | 2 +- Lib/test/test_interpreters/test_api.py | 32 ++++++++++------------- Modules/_xxsubinterpretersmodule.c | 36 +++++++++++++------------- 4 files changed, 33 insertions(+), 39 deletions(-) diff --git a/Lib/test/test__xxsubinterpreters.py b/Lib/test/test__xxsubinterpreters.py index 6251afcb606fea..c8c964f642f1cf 100644 --- a/Lib/test/test__xxsubinterpreters.py +++ b/Lib/test/test__xxsubinterpreters.py @@ -309,7 +309,7 @@ class IsRunningTests(TestBase): def test_main(self): main, *_ = _interpreters.get_main() - self.assertTrue(_interpreters.is_running(main, require_owned=False)) + self.assertTrue(_interpreters.is_running(main)) @unittest.skip('Fails on FreeBSD') def test_subinterpreter(self): diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 1e0995be09774d..35d6a209122a99 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2428,7 +2428,7 @@ def new_interp(config): expected = _interpreters.new_config('legacy') expected.gil = 'own' interpid, *_ = _interpreters.get_main() - config = _interpreters.get_config(interpid, require_owned=False) + config = _interpreters.get_config(interpid) self.assert_ns_equal(config, expected) with self.subTest('isolated'): diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index 2b6682e29bad21..df8800688ed682 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -1431,10 +1431,10 @@ def test_destroy(self): with self.subTest('from C-API'): interpid = _testinternalcapi.create_interpreter() with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.destroy(interpid) + _interpreters.destroy(interpid, require_owned=True) self.assertTrue( self.interp_exists(interpid)) - _interpreters.destroy(interpid, require_owned=False) + _interpreters.destroy(interpid) self.assertFalse( self.interp_exists(interpid)) @@ -1446,7 +1446,7 @@ def test_get_config(self): expected = _interpreters.new_config('legacy') expected.gil = 'own' interpid, *_ = _interpreters.get_main() - config = _interpreters.get_config(interpid, require_owned=False) + config = _interpreters.get_config(interpid) self.assert_ns_equal(config, expected) with self.subTest('isolated'): @@ -1465,8 +1465,8 @@ def test_get_config(self): orig = _interpreters.new_config('isolated') with self.interpreter_from_capi(orig) as interpid: with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.get_config(interpid) - config = _interpreters.get_config(interpid, require_owned=False) + _interpreters.get_config(interpid, require_owned=True) + config = _interpreters.get_config(interpid) self.assert_ns_equal(config, orig) @requires_test_modules @@ -1516,8 +1516,8 @@ def test_whence(self): def test_is_running(self): def check(interpid, expected): with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.is_running(interpid) - running = _interpreters.is_running(interpid, require_owned=False) + _interpreters.is_running(interpid, require_owned=True) + running = _interpreters.is_running(interpid) self.assertIs(running, expected) with self.subTest('from _interpreters (running)'): @@ -1585,12 +1585,9 @@ def test_exec(self): with self.subTest('from C-API'): with self.interpreter_from_capi() as interpid: with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.exec(interpid, 'raise Exception("it worked!")') - exc = _interpreters.exec( - interpid, - 'raise Exception("it worked!")', - require_owned=False, - ) + _interpreters.exec(interpid, 'raise Exception("it worked!")', + require_owned=True) + exc = _interpreters.exec(interpid, 'raise Exception("it worked!")') self.assertIsNot(exc, None) self.assertEqual(exc.msg, 'it worked!') @@ -1638,12 +1635,9 @@ def test_set___main___attrs(self): with self.subTest('from C-API'): with self.interpreter_from_capi() as interpid: with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.set___main___attrs(interpid, {'spam': True}) - _interpreters.set___main___attrs( - interpid, - {'spam': True}, - require_owned=False, - ) + _interpreters.set___main___attrs(interpid, {'spam': True}, + require_owned=True) + _interpreters.set___main___attrs(interpid, {'spam': True}) rc = _testinternalcapi.exec_interpreter( interpid, 'assert spam is True', diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 608e05a25e60c1..c7ec8f644afe97 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -809,7 +809,7 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) { static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *id; - int reqowned = 1; + int reqowned = 0; // XXX Use "L" for id? if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$p:destroy", kwlist, &id, &reqowned)) @@ -852,7 +852,7 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(destroy_doc, -"destroy(id, *, require_owned=True)\n\ +"destroy(id, *, require_owned=False)\n\ \n\ Destroy the identified interpreter.\n\ \n\ @@ -930,7 +930,7 @@ interp_set___main___attrs(PyObject *self, PyObject *args, PyObject *kwargs) { static char *kwlist[] = {"id", "updates", "require_owned", NULL}; PyObject *id, *updates; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "OO|$p:" MODULE_NAME_STR ".set___main___attrs", kwlist, &id, &updates, &reqowned)) @@ -979,7 +979,7 @@ interp_set___main___attrs(PyObject *self, PyObject *args, PyObject *kwargs) } PyDoc_STRVAR(set___main___attrs_doc, -"set___main___attrs(id, ns, *, require_owned=True)\n\ +"set___main___attrs(id, ns, *, require_owned=False)\n\ \n\ Bind the given attributes in the interpreter's __main__ module."); @@ -1091,7 +1091,7 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) static char *kwlist[] = {"id", "code", "shared", "require_owned", NULL}; PyObject *id, *code; PyObject *shared = NULL; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|O$p:" MODULE_NAME_STR ".exec", kwlist, &id, &code, &shared, &reqowned)) @@ -1128,7 +1128,7 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(exec_doc, -"exec(id, code, shared=None, *, require_owned=True)\n\ +"exec(id, code, shared=None, *, require_owned=False)\n\ \n\ Execute the provided code in the identified interpreter.\n\ This is equivalent to running the builtin exec() under the target\n\ @@ -1152,7 +1152,7 @@ interp_call(PyObject *self, PyObject *args, PyObject *kwds) PyObject *id, *callable; PyObject *args_obj = NULL; PyObject *kwargs_obj = NULL; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|OO$p:" MODULE_NAME_STR ".call", kwlist, &id, &callable, &args_obj, &kwargs_obj, @@ -1192,7 +1192,7 @@ interp_call(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(call_doc, -"call(id, callable, args=None, kwargs=None, *, require_owned=True)\n\ +"call(id, callable, args=None, kwargs=None, *, require_owned=False)\n\ \n\ Call the provided object in the identified interpreter.\n\ Pass the given args and kwargs, if possible.\n\ @@ -1209,7 +1209,7 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) static char *kwlist[] = {"id", "script", "shared", "require_owned", NULL}; PyObject *id, *script; PyObject *shared = NULL; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OU|O$p:" MODULE_NAME_STR ".run_string", kwlist, &id, &script, &shared, &reqowned)) @@ -1239,7 +1239,7 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(run_string_doc, -"run_string(id, script, shared=None, *, require_owned=True)\n\ +"run_string(id, script, shared=None, *, require_owned=False)\n\ \n\ Execute the provided string in the identified interpreter.\n\ \n\ @@ -1251,7 +1251,7 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) static char *kwlist[] = {"id", "func", "shared", "require_owned", NULL}; PyObject *id, *func; PyObject *shared = NULL; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|O$p:" MODULE_NAME_STR ".run_func", kwlist, &id, &func, &shared, &reqowned)) @@ -1283,7 +1283,7 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(run_func_doc, -"run_func(id, func, shared=None, *, require_owned=True)\n\ +"run_func(id, func, shared=None, *, require_owned=False)\n\ \n\ Execute the body of the provided function in the identified interpreter.\n\ Code objects are also supported. In both cases, closures and args\n\ @@ -1321,7 +1321,7 @@ interp_is_running(PyObject *self, PyObject *args, PyObject *kwds) { static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *id; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$p:is_running", kwlist, &id, &reqowned)) @@ -1342,7 +1342,7 @@ interp_is_running(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(is_running_doc, -"is_running(id, *, require_owned=True) -> bool\n\ +"is_running(id, *, require_owned=False) -> bool\n\ \n\ Return whether or not the identified interpreter is running."); @@ -1352,7 +1352,7 @@ interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) { static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *idobj = NULL; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$p:get_config", kwlist, &idobj, &reqowned)) @@ -1384,7 +1384,7 @@ interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(get_config_doc, -"get_config(id, *, require_owned=True) -> types.SimpleNamespace\n\ +"get_config(id, *, require_owned=False) -> types.SimpleNamespace\n\ \n\ Return a representation of the config used to initialize the interpreter."); @@ -1446,7 +1446,7 @@ interp_incref(PyObject *self, PyObject *args, PyObject *kwds) static char *kwlist[] = {"id", "implieslink", "require_owned", NULL}; PyObject *id; int implieslink = 0; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$pp:incref", kwlist, &id, &implieslink, &reqowned)) @@ -1474,7 +1474,7 @@ interp_decref(PyObject *self, PyObject *args, PyObject *kwds) { static char *kwlist[] = {"id", "require_owned", NULL}; PyObject *id; - int reqowned = 1; + int reqowned = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$p:decref", kwlist, &id, &reqowned)) { From f73b6f329c042f4714149c4d93c3520d10d02478 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 Apr 2024 18:08:53 -0600 Subject: [PATCH 10/18] Interpreter.owned -> Interpreter.whence --- Lib/test/support/interpreters/__init__.py | 57 ++++++----- Lib/test/test_interpreters/test_api.py | 112 ++++++++++++---------- Lib/test/test_interpreters/utils.py | 8 +- Modules/_xxsubinterpretersmodule.c | 68 ++++++------- 4 files changed, 137 insertions(+), 108 deletions(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index 9f68f157712a0d..0154c4d55a4e36 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -74,26 +74,26 @@ def __str__(self): def create(): """Return a new (idle) Python interpreter.""" id = _interpreters.create(reqrefs=True) - return Interpreter(id, _owned=True) + return Interpreter(id, _ownsref=True) def list_all(): """Return all existing interpreters.""" - return [Interpreter(id, _owned=owned) - for id, owned in _interpreters.list_all()] + return [Interpreter(id, _whence=whence) + for id, whence in _interpreters.list_all()] def get_current(): """Return the currently running interpreter.""" - id, owned = _interpreters.get_current() - return Interpreter(id, _owned=owned) + id, whence = _interpreters.get_current() + return Interpreter(id, _whence=whence) def get_main(): """Return the main interpreter.""" - id, owned = _interpreters.get_main() - assert owned is False - return Interpreter(id, _owned=owned) + id, whence = _interpreters.get_main() + assert whence == _interpreters.WHENCE_RUNTIME, repr(whence) + return Interpreter(id, _whence=whence) _known = weakref.WeakValueDictionary() @@ -104,21 +104,35 @@ class Interpreter: Attributes: "id" - the unique process-global ID number for the interpreter - "owned" - indicates whether or not the interpreter was created - by interpreters.create() + "whence" - indicates where the interpreter was created - If interp.owned is false then any method that modifies the - interpreter will fail, i.e. .close(), .prepare_main(), .exec(), - and .call() + If the interpreter wasn't created by this module + then any method that modifies the interpreter will fail, + i.e. .close(), .prepare_main(), .exec(), and .call() """ - def __new__(cls, id, /, _owned=None): + _WHENCE_TO_STR = { + _interpreters.WHENCE_UNKNOWN: 'unknown', + _interpreters.WHENCE_RUNTIME: 'runtime init', + _interpreters.WHENCE_LEGACY_CAPI: 'legacy C-API', + _interpreters.WHENCE_CAPI: 'C-API', + _interpreters.WHENCE_XI: 'cross-interpreter C-API', + _interpreters.WHENCE_STDLIB: '_interpreters module', + } + + def __new__(cls, id, /, _whence=None, _ownsref=None): # There is only one instance for any given ID. if not isinstance(id, int): raise TypeError(f'id must be an int, got {id!r}') id = int(id) - if _owned is None: - _owned = _interpreters.is_owned(id) + if _whence is None: + if _ownsref: + _whence = _interpreters.WHENCE_STDLIB + else: + _whence = _interpreters.whence(id) + assert _whence in cls._WHENCE_TO_STR, repr(_whence) + if _ownsref is None: + _ownsref = (_whence == _interpreters.WHENCE_STDLIB) try: self = _known[id] assert hasattr(self, '_ownsref') @@ -126,9 +140,9 @@ def __new__(cls, id, /, _owned=None): self = super().__new__(cls) _known[id] = self self._id = id - self._owned = _owned - self._ownsref = _owned - if _owned: + self._whence = _whence + self._ownsref = _ownsref + if _ownsref: # This may raise InterpreterNotFoundError: _interpreters.incref(id) return self @@ -163,10 +177,9 @@ def _decref(self): def id(self): return self._id - # XXX Is this the right name? @property - def owned(self): - return self._owned + def whence(self): + return self._WHENCE_TO_STR[self._whence] def is_running(self): """Return whether or not the identified interpreter is running.""" diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index df8800688ed682..f9a2dee75b9a15 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -20,6 +20,14 @@ ) +WHENCE_STR_UNKNOWN = 'unknown' +WHENCE_STR_RUNTIME = 'runtime init' +WHENCE_STR_LEGACY_CAPI = 'legacy C-API' +WHENCE_STR_CAPI = 'C-API' +WHENCE_STR_XI = 'cross-interpreter C-API' +WHENCE_STR_STDLIB = '_interpreters module' + + class ModuleTests(TestBase): def test_queue_aliases(self): @@ -172,11 +180,11 @@ def test_created_with_capi(self): text = self.run_temp_from_capi(f""" import {interpreters.__name__} as interpreters interp = interpreters.get_current() - print((interp.id, interp.owned)) + print((interp.id, interp.whence)) """) - interpid, owned = eval(text) + interpid, whence = eval(text) self.assertEqual(interpid, expected) - self.assertFalse(owned) + self.assertEqual(whence, WHENCE_STR_CAPI) class ListAllTests(TestBase): @@ -228,22 +236,22 @@ def test_created_with_capi(self): interpid4 = interpid3 + 1 interpid5 = interpid4 + 1 expected = [ - (mainid, False), - (interpid1, True), - (interpid2, True), - (interpid3, True), - (interpid4, False), - (interpid5, True), + (mainid, WHENCE_STR_RUNTIME), + (interpid1, WHENCE_STR_STDLIB), + (interpid2, WHENCE_STR_STDLIB), + (interpid3, WHENCE_STR_STDLIB), + (interpid4, WHENCE_STR_CAPI), + (interpid5, WHENCE_STR_STDLIB), ] expected2 = expected[:-2] text = self.run_temp_from_capi(f""" import {interpreters.__name__} as interpreters interp = interpreters.create() print( - [(i.id, i.owned) for i in interpreters.list_all()]) + [(i.id, i.whence) for i in interpreters.list_all()]) """) res = eval(text) - res2 = [(i.id, i.owned) for i in interpreters.list_all()] + res2 = [(i.id, i.whence) for i in interpreters.list_all()] self.assertEqual(res, expected) self.assertEqual(res2, expected2) @@ -299,31 +307,37 @@ def test_id_readonly(self): with self.assertRaises(AttributeError): interp.id = 1_000_000 - def test_owned(self): + def test_whence(self): main = interpreters.get_main() interp = interpreters.create() with self.subTest('main'): - self.assertFalse(main.owned) + self.assertEqual(main.whence, WHENCE_STR_RUNTIME) with self.subTest('from _interpreters'): - self.assertTrue(interp.owned) + self.assertEqual(interp.whence, WHENCE_STR_STDLIB) with self.subTest('from C-API'): text = self.run_temp_from_capi(f""" import {interpreters.__name__} as interpreters interp = interpreters.get_current() - print(interp.owned) + print(repr(interp.whence)) """) - owned = eval(text) - self.assertFalse(owned) + whence = eval(text) + self.assertEqual(whence, WHENCE_STR_CAPI) with self.subTest('readonly'): - for value in (True, False): + for value in [ + None, + WHENCE_STR_UNKNOWN, + WHENCE_STR_RUNTIME, + WHENCE_STR_STDLIB, + WHENCE_STR_CAPI, + ]: with self.assertRaises(AttributeError): - interp.owned = value + interp.whence = value with self.assertRaises(AttributeError): - main.owned = value + main.whence = value def test_hashable(self): interp = interpreters.create() @@ -612,7 +626,7 @@ def test_created_with_capi(self): self.interp_exists(interpid)) # The rest would be skipped until we deal with running threads when - # interp.close() is called. However, the "owned" restrictions + # interp.close() is called. However, the "whence" restrictions # trigger first. with self.subTest('running, but not __main__ (from other)'): @@ -1263,38 +1277,35 @@ def test_new_config(self): _interpreters.new_config(gil=value) def test_get_main(self): - interpid, owned = _interpreters.get_main() + interpid, whence = _interpreters.get_main() self.assertEqual(interpid, 0) - self.assertFalse(owned) - self.assertFalse( - _interpreters.is_owned(interpid)) + self.assertEqual(whence, _interpreters.WHENCE_RUNTIME) + self.assertEqual( + _interpreters.whence(interpid), + _interpreters.WHENCE_RUNTIME) def test_get_current(self): with self.subTest('main'): main, *_ = _interpreters.get_main() - interpid, owned = _interpreters.get_current() + interpid, whence = _interpreters.get_current() self.assertEqual(interpid, main) - self.assertFalse(owned) + self.assertEqual(whence, _interpreters.WHENCE_RUNTIME) script = f""" import {_interpreters.__name__} as _interpreters - interpid, owned = _interpreters.get_current() - print(interpid, owned) + interpid, whence = _interpreters.get_current() + print((interpid, whence)) """ def parse_stdout(text): - parts = text.split() - assert len(parts) == 2, parts - interpid, owned = parts - interpid = int(interpid) - owned = eval(owned) - return interpid, owned + interpid, whence = eval(text) + return interpid, whence with self.subTest('from _interpreters'): orig = _interpreters.create() text = self.run_and_capture(orig, script) - interpid, owned = parse_stdout(text) + interpid, whence = parse_stdout(text) self.assertEqual(interpid, orig) - self.assertTrue(owned) + self.assertEqual(whence, _interpreters.WHENCE_STDLIB) with self.subTest('from C-API'): last = 0 @@ -1302,9 +1313,9 @@ def parse_stdout(text): last = max(last, id) expected = last + 1 text = self.run_temp_from_capi(script) - interpid, owned = parse_stdout(text) + interpid, whence = parse_stdout(text) self.assertEqual(interpid, expected) - self.assertFalse(owned) + self.assertEqual(whence, _interpreters.WHENCE_CAPI) def test_list_all(self): mainid, *_ = _interpreters.get_main() @@ -1312,10 +1323,10 @@ def test_list_all(self): interpid2 = _interpreters.create() interpid3 = _interpreters.create() expected = [ - (mainid, False), - (interpid1, True), - (interpid2, True), - (interpid3, True), + (mainid, _interpreters.WHENCE_RUNTIME), + (interpid1, _interpreters.WHENCE_STDLIB), + (interpid2, _interpreters.WHENCE_STDLIB), + (interpid3, _interpreters.WHENCE_STDLIB), ] with self.subTest('main'): @@ -1336,11 +1347,11 @@ def test_list_all(self): interpid4 = interpid3 + 1 interpid5 = interpid4 + 1 expected2 = expected + [ - (interpid4, False), - (interpid5, True), + (interpid4, _interpreters.WHENCE_CAPI), + (interpid5, _interpreters.WHENCE_STDLIB), ] expected3 = expected + [ - (interpid5, True), + (interpid5, _interpreters.WHENCE_STDLIB), ] text = self.run_temp_from_capi(f""" import {_interpreters.__name__} as _interpreters @@ -1404,10 +1415,11 @@ def test_create(self): with self.assertRaises(ValueError): _interpreters.create(orig) - with self.subTest('is owned'): + with self.subTest('whence'): interpid = _interpreters.create() - self.assertTrue( - _interpreters.is_owned(interpid)) + self.assertEqual( + _interpreters.whence(interpid), + _interpreters.WHENCE_STDLIB) @requires_test_modules def test_destroy(self): @@ -1479,7 +1491,7 @@ def test_whence(self): with self.subTest('stdlib'): interpid = _interpreters.create() whence = _interpreters.whence(interpid) - self.assertEqual(whence, _interpreters.WHENCE_XI) + self.assertEqual(whence, _interpreters.WHENCE_STDLIB) for orig, name in { # XXX Also check WHENCE_UNKNOWN. diff --git a/Lib/test/test_interpreters/utils.py b/Lib/test/test_interpreters/utils.py index 14ea43d4710c79..08768c07c28ebf 100644 --- a/Lib/test/test_interpreters/utils.py +++ b/Lib/test/test_interpreters/utils.py @@ -511,7 +511,7 @@ def run_and_capture(self, interp, script): def interp_exists(self, interpid): try: - _interpreters.is_owned(interpid) + _interpreters.whence(interpid) except _interpreters.InterpreterNotFoundError: return False else: @@ -553,7 +553,11 @@ def interpreter_from_capi(self, config=None, whence=None): @contextlib.contextmanager def interpreter_obj_from_capi(self, config='legacy'): with self.interpreter_from_capi(config) as interpid: - interp = interpreters.Interpreter(interpid, _owned=False) + interp = interpreters.Interpreter( + interpid, + _whence=_interpreters.WHENCE_CAPI, + _ownsref=False, + ) yield interp, interpid @contextlib.contextmanager diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index c7ec8f644afe97..eb1d5cc134dc45 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -658,6 +658,24 @@ _globals_fini(void) /* module level code ********************************************************/ +#define _PyInterpreterState_WHENCE_STDLIB 5 + +static long +get_whence(PyInterpreterState *interp) +{ + long whence = _PyInterpreterState_GetWhence(interp); + if (whence == _PyInterpreterState_WHENCE_XI) { + if (is_owned(&_globals.owned, interp)) { + whence = _PyInterpreterState_WHENCE_STDLIB; + } + } + else { + assert(!is_owned(&_globals.owned, interp)); + } + return whence; +} + + static PyInterpreterState * resolve_interp(PyObject *idobj, int reqowned, const char *op) { @@ -672,7 +690,7 @@ resolve_interp(PyObject *idobj, int reqowned, const char *op) } } - if (reqowned && !is_owned(&_globals.owned, interp)) { + if (reqowned && get_whence(interp) != _PyInterpreterState_WHENCE_STDLIB) { if (idobj == NULL) { PyErr_Format(PyExc_InterpreterError, "cannot %s unrecognized current interpreter", op); @@ -695,9 +713,15 @@ get_summary(PyInterpreterState *interp) if (idobj == NULL) { return NULL; } - PyObject *owned = is_owned(&_globals.owned, interp) ? Py_True : Py_False; - PyObject *res = PyTuple_Pack(2, idobj, owned); + PyObject *whenceobj = PyLong_FromLong( + get_whence(interp)); + if (whenceobj == NULL) { + Py_DECREF(idobj); + return NULL; + } + PyObject *res = PyTuple_Pack(2, idobj, whenceobj); Py_DECREF(idobj); + Py_DECREF(whenceobj); return res; } @@ -891,7 +915,7 @@ interp_list_all(PyObject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(list_all_doc, -"list_all() -> [(ID, owned)]\n\ +"list_all() -> [(ID, whence)]\n\ \n\ Return a list containing the ID of every existing interpreter."); @@ -907,7 +931,7 @@ interp_get_current(PyObject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(get_current_doc, -"get_current() -> (ID, owned)\n\ +"get_current() -> (ID, whence)\n\ \n\ Return the ID of current interpreter."); @@ -920,7 +944,7 @@ interp_get_main(PyObject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(get_main_doc, -"get_main() -> (ID, owned)\n\ +"get_main() -> (ID, whence)\n\ \n\ Return the ID of main interpreter."); @@ -1405,7 +1429,7 @@ interp_whence(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - long whence = _PyInterpreterState_GetWhence(interp); + long whence = get_whence(interp); return PyLong_FromLong(whence); } @@ -1415,31 +1439,6 @@ PyDoc_STRVAR(whence_doc, Return an identifier for where the interpreter was created."); -static PyObject * -interp_is_owned(PyObject *self, PyObject *args, PyObject *kwds) -{ - static char *kwlist[] = {"id", NULL}; - PyObject *id; - if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O:is_owned", kwlist, &id)) - { - return NULL; - } - - PyInterpreterState *interp = look_up_interp(id); - if (interp == NULL) { - return NULL; - } - - return is_owned(&_globals.owned, interp) ? Py_True : Py_False; -} - -PyDoc_STRVAR(is_owned_doc, -"is_owned(id) -> bool\n\ -\n\ -Return True if the interpreter was created by this module."); - - static PyObject * interp_incref(PyObject *self, PyObject *args, PyObject *kwds) { @@ -1575,8 +1574,8 @@ static PyMethodDef module_functions[] = { {"get_main", interp_get_main, METH_NOARGS, get_main_doc}, - {"is_owned", _PyCFunction_CAST(interp_is_owned), - METH_VARARGS | METH_KEYWORDS, is_owned_doc}, + {"whence", _PyCFunction_CAST(interp_whence), + METH_VARARGS | METH_KEYWORDS, whence_doc}, {"is_running", _PyCFunction_CAST(interp_is_running), METH_VARARGS | METH_KEYWORDS, is_running_doc}, {"get_config", _PyCFunction_CAST(interp_get_config), @@ -1637,6 +1636,7 @@ module_exec(PyObject *mod) ADD_WHENCE(LEGACY_CAPI) ADD_WHENCE(CAPI) ADD_WHENCE(XI) + ADD_WHENCE(STDLIB) #undef ADD_WHENCE // exceptions From 7e6432c3ea4e3bdc617b7edbdc740030f8d49d95 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 Apr 2024 18:22:51 -0600 Subject: [PATCH 11/18] require_owned -> restrict --- Lib/test/support/interpreters/__init__.py | 11 ++- Lib/test/test_interpreters/test_api.py | 10 +-- Modules/_xxsubinterpretersmodule.c | 100 +++++++++++----------- 3 files changed, 60 insertions(+), 61 deletions(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index 0154c4d55a4e36..18256a0b8265aa 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -183,8 +183,7 @@ def whence(self): def is_running(self): """Return whether or not the identified interpreter is running.""" - # require_owned is okay since this doesn't modify the interpreter. - return _interpreters.is_running(self._id, require_owned=False) + return _interpreters.is_running(self._id) # Everything past here is available only to interpreters created by # interpreters.create(). @@ -195,7 +194,7 @@ def close(self): Attempting to destroy the current interpreter results in an InterpreterError. """ - return _interpreters.destroy(self._id, require_owned=True) + return _interpreters.destroy(self._id, restrict=True) def prepare_main(self, ns=None, /, **kwargs): """Bind the given values into the interpreter's __main__. @@ -203,7 +202,7 @@ def prepare_main(self, ns=None, /, **kwargs): The values must be shareable. """ ns = dict(ns, **kwargs) if ns is not None else kwargs - _interpreters.set___main___attrs(self._id, ns, require_owned=True) + _interpreters.set___main___attrs(self._id, ns, restrict=True) def exec(self, code, /): """Run the given source code in the interpreter. @@ -223,7 +222,7 @@ def exec(self, code, /): that time, the previous interpreter is allowed to run in other threads. """ - excinfo = _interpreters.exec(self._id, code, require_owned=True) + excinfo = _interpreters.exec(self._id, code, restrict=True) if excinfo is not None: raise ExecutionFailed(excinfo) @@ -243,7 +242,7 @@ def call(self, callable, /): # XXX Support args and kwargs. # XXX Support arbitrary callables. # XXX Support returning the return value (e.g. via pickle). - excinfo = _interpreters.call(self._id, callable, require_owned=True) + excinfo = _interpreters.call(self._id, callable, restrict=True) if excinfo is not None: raise ExecutionFailed(excinfo) diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index f9a2dee75b9a15..dd59f5df9a37be 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -1443,7 +1443,7 @@ def test_destroy(self): with self.subTest('from C-API'): interpid = _testinternalcapi.create_interpreter() with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.destroy(interpid, require_owned=True) + _interpreters.destroy(interpid, restrict=True) self.assertTrue( self.interp_exists(interpid)) _interpreters.destroy(interpid) @@ -1477,7 +1477,7 @@ def test_get_config(self): orig = _interpreters.new_config('isolated') with self.interpreter_from_capi(orig) as interpid: with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.get_config(interpid, require_owned=True) + _interpreters.get_config(interpid, restrict=True) config = _interpreters.get_config(interpid) self.assert_ns_equal(config, orig) @@ -1528,7 +1528,7 @@ def test_whence(self): def test_is_running(self): def check(interpid, expected): with self.assertRaisesRegex(InterpreterError, 'unrecognized'): - _interpreters.is_running(interpid, require_owned=True) + _interpreters.is_running(interpid, restrict=True) running = _interpreters.is_running(interpid) self.assertIs(running, expected) @@ -1598,7 +1598,7 @@ def test_exec(self): with self.interpreter_from_capi() as interpid: with self.assertRaisesRegex(InterpreterError, 'unrecognized'): _interpreters.exec(interpid, 'raise Exception("it worked!")', - require_owned=True) + restrict=True) exc = _interpreters.exec(interpid, 'raise Exception("it worked!")') self.assertIsNot(exc, None) self.assertEqual(exc.msg, 'it worked!') @@ -1648,7 +1648,7 @@ def test_set___main___attrs(self): with self.interpreter_from_capi() as interpid: with self.assertRaisesRegex(InterpreterError, 'unrecognized'): _interpreters.set___main___attrs(interpid, {'spam': True}, - require_owned=True) + restrict=True) _interpreters.set___main___attrs(interpid, {'spam': True}) rc = _testinternalcapi.exec_interpreter( interpid, diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index eb1d5cc134dc45..65a9c639d81159 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -677,7 +677,7 @@ get_whence(PyInterpreterState *interp) static PyInterpreterState * -resolve_interp(PyObject *idobj, int reqowned, const char *op) +resolve_interp(PyObject *idobj, int restricted, const char *op) { PyInterpreterState *interp; if (idobj == NULL) { @@ -690,7 +690,7 @@ resolve_interp(PyObject *idobj, int reqowned, const char *op) } } - if (reqowned && get_whence(interp) != _PyInterpreterState_WHENCE_STDLIB) { + if (restricted && get_whence(interp) != _PyInterpreterState_WHENCE_STDLIB) { if (idobj == NULL) { PyErr_Format(PyExc_InterpreterError, "cannot %s unrecognized current interpreter", op); @@ -831,18 +831,18 @@ is \"isolated\"."); static PyObject * interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "require_owned", NULL}; + static char *kwlist[] = {"id", "restrict", NULL}; PyObject *id; - int reqowned = 0; + int restricted = 0; // XXX Use "L" for id? if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O|$p:destroy", kwlist, &id, &reqowned)) + "O|$p:destroy", kwlist, &id, &restricted)) { return NULL; } // Look up the interpreter. - PyInterpreterState *interp = resolve_interp(id, reqowned, "destroy"); + PyInterpreterState *interp = resolve_interp(id, restricted, "destroy"); if (interp == NULL) { return NULL; } @@ -876,7 +876,7 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(destroy_doc, -"destroy(id, *, require_owned=False)\n\ +"destroy(id, *, restrict=False)\n\ \n\ Destroy the identified interpreter.\n\ \n\ @@ -952,18 +952,18 @@ Return the ID of main interpreter."); static PyObject * interp_set___main___attrs(PyObject *self, PyObject *args, PyObject *kwargs) { - static char *kwlist[] = {"id", "updates", "require_owned", NULL}; + static char *kwlist[] = {"id", "updates", "restrict", NULL}; PyObject *id, *updates; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "OO|$p:" MODULE_NAME_STR ".set___main___attrs", - kwlist, &id, &updates, &reqowned)) + kwlist, &id, &updates, &restricted)) { return NULL; } // Look up the interpreter. - PyInterpreterState *interp = resolve_interp(id, reqowned, + PyInterpreterState *interp = resolve_interp(id, restricted, "update __main__ for"); if (interp == NULL) { return NULL; @@ -1003,7 +1003,7 @@ interp_set___main___attrs(PyObject *self, PyObject *args, PyObject *kwargs) } PyDoc_STRVAR(set___main___attrs_doc, -"set___main___attrs(id, ns, *, require_owned=False)\n\ +"set___main___attrs(id, ns, *, restrict=False)\n\ \n\ Bind the given attributes in the interpreter's __main__ module."); @@ -1112,18 +1112,18 @@ _interp_exec(PyObject *self, PyInterpreterState *interp, static PyObject * interp_exec(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "code", "shared", "require_owned", NULL}; + static char *kwlist[] = {"id", "code", "shared", "restrict", NULL}; PyObject *id, *code; PyObject *shared = NULL; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|O$p:" MODULE_NAME_STR ".exec", kwlist, - &id, &code, &shared, &reqowned)) + &id, &code, &shared, &restricted)) { return NULL; } - PyInterpreterState *interp = resolve_interp(id, reqowned, "exec code for"); + PyInterpreterState *interp = resolve_interp(id, restricted, "exec code for"); if (interp == NULL) { return NULL; } @@ -1152,7 +1152,7 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(exec_doc, -"exec(id, code, shared=None, *, require_owned=False)\n\ +"exec(id, code, shared=None, *, restrict=False)\n\ \n\ Execute the provided code in the identified interpreter.\n\ This is equivalent to running the builtin exec() under the target\n\ @@ -1172,20 +1172,20 @@ static PyObject * interp_call(PyObject *self, PyObject *args, PyObject *kwds) { static char *kwlist[] = {"id", "callable", "args", "kwargs", - "require_owned", NULL}; + "restrict", NULL}; PyObject *id, *callable; PyObject *args_obj = NULL; PyObject *kwargs_obj = NULL; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|OO$p:" MODULE_NAME_STR ".call", kwlist, &id, &callable, &args_obj, &kwargs_obj, - &reqowned)) + &restricted)) { return NULL; } - PyInterpreterState *interp = resolve_interp(id, reqowned, "make a call in"); + PyInterpreterState *interp = resolve_interp(id, restricted, "make a call in"); if (interp == NULL) { return NULL; } @@ -1216,7 +1216,7 @@ interp_call(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(call_doc, -"call(id, callable, args=None, kwargs=None, *, require_owned=False)\n\ +"call(id, callable, args=None, kwargs=None, *, restrict=False)\n\ \n\ Call the provided object in the identified interpreter.\n\ Pass the given args and kwargs, if possible.\n\ @@ -1230,18 +1230,18 @@ is ignored, including its __globals__ dict."); static PyObject * interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "script", "shared", "require_owned", NULL}; + static char *kwlist[] = {"id", "script", "shared", "restrict", NULL}; PyObject *id, *script; PyObject *shared = NULL; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OU|O$p:" MODULE_NAME_STR ".run_string", - kwlist, &id, &script, &shared, &reqowned)) + kwlist, &id, &script, &shared, &restricted)) { return NULL; } - PyInterpreterState *interp = resolve_interp(id, reqowned, "run a string in"); + PyInterpreterState *interp = resolve_interp(id, restricted, "run a string in"); if (interp == NULL) { return NULL; } @@ -1263,7 +1263,7 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(run_string_doc, -"run_string(id, script, shared=None, *, require_owned=False)\n\ +"run_string(id, script, shared=None, *, restrict=False)\n\ \n\ Execute the provided string in the identified interpreter.\n\ \n\ @@ -1272,18 +1272,18 @@ Execute the provided string in the identified interpreter.\n\ static PyObject * interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "func", "shared", "require_owned", NULL}; + static char *kwlist[] = {"id", "func", "shared", "restrict", NULL}; PyObject *id, *func; PyObject *shared = NULL; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|O$p:" MODULE_NAME_STR ".run_func", - kwlist, &id, &func, &shared, &reqowned)) + kwlist, &id, &func, &shared, &restricted)) { return NULL; } - PyInterpreterState *interp = resolve_interp(id, reqowned, + PyInterpreterState *interp = resolve_interp(id, restricted, "run a function in"); if (interp == NULL) { return NULL; @@ -1307,7 +1307,7 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(run_func_doc, -"run_func(id, func, shared=None, *, require_owned=False)\n\ +"run_func(id, func, shared=None, *, restrict=False)\n\ \n\ Execute the body of the provided function in the identified interpreter.\n\ Code objects are also supported. In both cases, closures and args\n\ @@ -1343,17 +1343,17 @@ False otherwise."); static PyObject * interp_is_running(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "require_owned", NULL}; + static char *kwlist[] = {"id", "restrict", NULL}; PyObject *id; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$p:is_running", kwlist, - &id, &reqowned)) + &id, &restricted)) { return NULL; } - PyInterpreterState *interp = resolve_interp(id, reqowned, + PyInterpreterState *interp = resolve_interp(id, restricted, "check if running for"); if (interp == NULL) { return NULL; @@ -1366,7 +1366,7 @@ interp_is_running(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(is_running_doc, -"is_running(id, *, require_owned=False) -> bool\n\ +"is_running(id, *, restrict=False) -> bool\n\ \n\ Return whether or not the identified interpreter is running."); @@ -1374,12 +1374,12 @@ Return whether or not the identified interpreter is running."); static PyObject * interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "require_owned", NULL}; + static char *kwlist[] = {"id", "restrict", NULL}; PyObject *idobj = NULL; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$p:get_config", kwlist, - &idobj, &reqowned)) + &idobj, &restricted)) { return NULL; } @@ -1387,7 +1387,7 @@ interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) idobj = NULL; } - PyInterpreterState *interp = resolve_interp(idobj, reqowned, + PyInterpreterState *interp = resolve_interp(idobj, restricted, "get the config of"); if (interp == NULL) { return NULL; @@ -1408,7 +1408,7 @@ interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) } PyDoc_STRVAR(get_config_doc, -"get_config(id, *, require_owned=False) -> types.SimpleNamespace\n\ +"get_config(id, *, restrict=False) -> types.SimpleNamespace\n\ \n\ Return a representation of the config used to initialize the interpreter."); @@ -1442,18 +1442,18 @@ Return an identifier for where the interpreter was created."); static PyObject * interp_incref(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "implieslink", "require_owned", NULL}; + static char *kwlist[] = {"id", "implieslink", "restrict", NULL}; PyObject *id; int implieslink = 0; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|$pp:incref", kwlist, - &id, &implieslink, &reqowned)) + &id, &implieslink, &restricted)) { return NULL; } - PyInterpreterState *interp = resolve_interp(id, reqowned, "incref"); + PyInterpreterState *interp = resolve_interp(id, restricted, "incref"); if (interp == NULL) { return NULL; } @@ -1471,16 +1471,16 @@ interp_incref(PyObject *self, PyObject *args, PyObject *kwds) static PyObject * interp_decref(PyObject *self, PyObject *args, PyObject *kwds) { - static char *kwlist[] = {"id", "require_owned", NULL}; + static char *kwlist[] = {"id", "restrict", NULL}; PyObject *id; - int reqowned = 0; + int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "O|$p:decref", kwlist, &id, &reqowned)) + "O|$p:decref", kwlist, &id, &restricted)) { return NULL; } - PyInterpreterState *interp = resolve_interp(id, reqowned, "decref"); + PyInterpreterState *interp = resolve_interp(id, restricted, "decref"); if (interp == NULL) { return NULL; } From 19b68ebd15f9930fd4c579ae097bbb7819550b25 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Apr 2024 10:38:52 -0600 Subject: [PATCH 12/18] Add PyInterpreterState._whence. --- Include/internal/pycore_crossinterp.h | 1 + Include/internal/pycore_interp.h | 3 ++- Modules/_testinternalcapi.c | 4 ++-- Modules/_xxsubinterpretersmodule.c | 17 ++++------------- Python/crossinterp.c | 8 ++++++-- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/Include/internal/pycore_crossinterp.h b/Include/internal/pycore_crossinterp.h index 64d881dbab7357..2dd165eae74850 100644 --- a/Include/internal/pycore_crossinterp.h +++ b/Include/internal/pycore_crossinterp.h @@ -325,6 +325,7 @@ PyAPI_FUNC(int) _PyXI_HasCapturedException(_PyXI_session *session); // Export for _testinternalcapi shared extension PyAPI_FUNC(PyInterpreterState *) _PyXI_NewInterpreter( PyInterpreterConfig *config, + long *maybe_whence, PyThreadState **p_tstate, PyThreadState **p_save_tstate); PyAPI_FUNC(void) _PyXI_EndInterpreter( diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index c96a9e40e4274a..17ec665c7f6a25 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -109,7 +109,8 @@ struct _is { #define _PyInterpreterState_WHENCE_LEGACY_CAPI 2 #define _PyInterpreterState_WHENCE_CAPI 3 #define _PyInterpreterState_WHENCE_XI 4 -#define _PyInterpreterState_WHENCE_MAX 4 +#define _PyInterpreterState_WHENCE_STDLIB 5 +#define _PyInterpreterState_WHENCE_MAX 5 long _whence; /* Has been initialized to a safe state. diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 538e4ceafd1743..36dad024d31c95 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1394,7 +1394,7 @@ static PyInterpreterState * _new_interpreter(PyInterpreterConfig *config, long whence) { if (whence == _PyInterpreterState_WHENCE_XI) { - return _PyXI_NewInterpreter(config, NULL, NULL); + return _PyXI_NewInterpreter(config, &whence, NULL, NULL); } PyObject *exc = NULL; PyInterpreterState *interp = NULL; @@ -1610,7 +1610,7 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) /* Create an interpreter, staying switched to it. */ PyInterpreterState *interp = \ - _PyXI_NewInterpreter(&config, &tstate, &save_tstate); + _PyXI_NewInterpreter(&config, NULL, &tstate, &save_tstate); if (interp == NULL) { return NULL; } diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 65a9c639d81159..da195ed17e5442 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -658,21 +658,10 @@ _globals_fini(void) /* module level code ********************************************************/ -#define _PyInterpreterState_WHENCE_STDLIB 5 - static long get_whence(PyInterpreterState *interp) { - long whence = _PyInterpreterState_GetWhence(interp); - if (whence == _PyInterpreterState_WHENCE_XI) { - if (is_owned(&_globals.owned, interp)) { - whence = _PyInterpreterState_WHENCE_STDLIB; - } - } - else { - assert(!is_owned(&_globals.owned, interp)); - } - return whence; + return _PyInterpreterState_GetWhence(interp); } @@ -786,7 +775,9 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = _PyXI_NewInterpreter(&config, NULL, NULL); + long whence = _PyInterpreterState_WHENCE_STDLIB; + PyInterpreterState *interp = \ + _PyXI_NewInterpreter(&config, &whence, NULL, NULL); if (interp == NULL) { // XXX Move the chained exception to interpreters.create()? PyObject *exc = PyErr_GetRaisedException(); diff --git a/Python/crossinterp.c b/Python/crossinterp.c index fb0dae0bbb8f75..b77cbf29a6818c 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -1822,7 +1822,7 @@ _PyXI_FiniTypes(PyInterpreterState *interp) /*************/ PyInterpreterState * -_PyXI_NewInterpreter(PyInterpreterConfig *config, +_PyXI_NewInterpreter(PyInterpreterConfig *config, long *maybe_whence, PyThreadState **p_tstate, PyThreadState **p_save_tstate) { PyThreadState *save_tstate = PyThreadState_Swap(NULL); @@ -1845,7 +1845,11 @@ _PyXI_NewInterpreter(PyInterpreterConfig *config, assert(tstate != NULL); PyInterpreterState *interp = PyThreadState_GetInterpreter(tstate); - _PyInterpreterState_SetWhence(interp, _PyInterpreterState_WHENCE_XI); + long whence = _PyInterpreterState_WHENCE_XI; + if (maybe_whence != NULL) { + whence = *maybe_whence; + } + _PyInterpreterState_SetWhence(interp, whence); if (p_tstate != NULL) { // We leave the new thread state as the current one. From d713ecff3ec4553c51160542e83d672a9ccd38c7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Apr 2024 16:31:55 -0600 Subject: [PATCH 13/18] Drop the module's global state. --- Modules/_xxsubinterpretersmodule.c | 179 ----------------------------- 1 file changed, 179 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index da195ed17e5442..d6d8256f00c258 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -489,173 +489,6 @@ _run_in_interpreter(PyInterpreterState *interp, } -/* "owned" interpreters *****************************************************/ - -struct owned_id { - int64_t id; - struct owned_id *next; -}; - -typedef struct { - PyMutex mutex; - struct owned_id *head; - Py_ssize_t count; -} owned_ids; - -static int -init_owned(owned_ids *owned) -{ - return 0; -} - -static void -clear_owned(owned_ids *owned) -{ - PyMutex_Lock(&owned->mutex); - struct owned_id *cur = owned->head; - Py_ssize_t count = 0; - while (cur != NULL) { - struct owned_id *next = cur->next; - PyMem_RawFree(cur); - cur = next; - count += 1; - } - assert(count == owned->count); - owned->head = NULL; - owned->count = 0; - PyMutex_Unlock(&owned->mutex); -} - -static struct owned_id * -_find_owned(owned_ids *owned, int64_t interpid, struct owned_id **p_prev) -{ - // The caller must manage the lock. - if (owned->head == NULL) { - return NULL; - } - - struct owned_id *prev = NULL; - struct owned_id *cur = owned->head; - while (cur != NULL) { - if (cur->id == interpid) { - break; - } - prev = cur; - cur = cur->next; - } - if (cur == NULL) { - return NULL; - } - - if (p_prev != NULL) { - *p_prev = prev; - } - return cur; -} - -static void -add_owned(owned_ids *owned, PyInterpreterState *interp) -{ - PyMutex_Lock(&owned->mutex); - int64_t id = PyInterpreterState_GetID(interp); - struct owned_id *new = PyMem_RawMalloc(sizeof(struct owned_id)); - assert(new != NULL); - new->id = id; - new->next = owned->head; - owned->head = new; - owned->count += 1; - PyMutex_Unlock(&owned->mutex); -} - -static void -drop_owned(owned_ids *owned, int64_t interpid) -{ - PyMutex_Lock(&owned->mutex); - - struct owned_id *prev; - struct owned_id *found = _find_owned(owned, interpid, &prev); - if (found != NULL) { - if (prev == NULL) { - assert(found == owned->head); - owned->head = found->next; - } - else { - assert(found != owned->head); - prev->next = found->next; - } - PyMem_RawFree(found); - owned->count -= 1; - } - - PyMutex_Unlock(&owned->mutex); -} - -static int -is_owned(owned_ids *owned, PyInterpreterState *interp) -{ - int64_t interpid = PyInterpreterState_GetID(interp); - PyMutex_Lock(&owned->mutex); - - struct owned_id *found = _find_owned(owned, interpid, NULL); - int res = found != NULL; - - PyMutex_Unlock(&owned->mutex); - return res; -} - - -/* global state *************************************************************/ - -/* globals is the process-global state for the module. It holds all - the data that we need to share between interpreters, so it cannot - hold PyObject values. */ -static struct globals { - PyMutex mutex; - int module_count; - owned_ids owned; -} _globals = {0}; - -static int -_globals_init(void) -{ - int res = -1; - PyMutex_Lock(&_globals.mutex); - - _globals.module_count++; - if (_globals.module_count > 1) { - // Already initialized. - res = 0; - goto finally; - } - - if (init_owned(&_globals.owned) < 0) { - goto finally; - } - - res = 0; - -finally: - PyMutex_Unlock(&_globals.mutex); - return res; -} - -static void -_globals_fini(void) -{ - PyMutex_Lock(&_globals.mutex); - - _globals.module_count--; - if (_globals.module_count > 0) { - goto finally; - } - - clear_owned(&_globals.owned); - -finally: - PyMutex_Unlock(&_globals.mutex); -} - - /* module level code ********************************************************/ static long @@ -793,8 +626,6 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - add_owned(&_globals.owned, interp); - if (reqrefs) { // Decref to 0 will destroy the interpreter. _PyInterpreterState_RequireIDRef(interp, 1); @@ -837,7 +668,6 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) if (interp == NULL) { return NULL; } - int64_t interpid = PyInterpreterState_GetID(interp); // Ensure we don't try to destroy the current interpreter. PyInterpreterState *current = _get_current_interp(); @@ -861,8 +691,6 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) // Destroy the interpreter. _PyXI_EndInterpreter(interp, NULL, NULL); - drop_owned(&_globals.owned, interpid); - Py_RETURN_NONE; } @@ -1612,10 +1440,6 @@ module_exec(PyObject *mod) PyInterpreterState *interp = PyInterpreterState_Get(); module_state *state = get_module_state(mod); - if (_globals_init() != 0) { - return -1; - } - #define ADD_WHENCE(NAME) \ if (PyModule_AddIntConstant(mod, "WHENCE_" #NAME, \ _PyInterpreterState_WHENCE_##NAME) < 0) \ @@ -1650,7 +1474,6 @@ module_exec(PyObject *mod) return 0; error: - _globals_fini(); return -1; } @@ -1684,8 +1507,6 @@ module_free(void *mod) module_state *state = get_module_state(mod); assert(state != NULL); clear_module_state(state); - - _globals_fini(); } static struct PyModuleDef moduledef = { From fe362d18d4f7ceda7d9bd71e3dfb23392081d530 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Apr 2024 16:39:56 -0600 Subject: [PATCH 14/18] Drop a duplicate module func def. --- Modules/_xxsubinterpretersmodule.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index d6d8256f00c258..fb831bc0de80a3 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -1393,8 +1393,6 @@ static PyMethodDef module_functions[] = { {"get_main", interp_get_main, METH_NOARGS, get_main_doc}, - {"whence", _PyCFunction_CAST(interp_whence), - METH_VARARGS | METH_KEYWORDS, whence_doc}, {"is_running", _PyCFunction_CAST(interp_is_running), METH_VARARGS | METH_KEYWORDS, is_running_doc}, {"get_config", _PyCFunction_CAST(interp_get_config), From cd7d5ee63a1b11214926eaf3a049bbdb248ff430 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Apr 2024 17:01:15 -0600 Subject: [PATCH 15/18] Require the interpreter to be ready for most operations. --- Include/internal/pycore_interp.h | 2 + Modules/_xxsubinterpretersmodule.c | 61 ++++++++++++++++++++++-------- Python/crossinterp.c | 2 +- Python/pystate.c | 7 ++++ 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 17ec665c7f6a25..d38959e3ce4ec5 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -317,6 +317,8 @@ PyAPI_FUNC(int) _PyInterpreterState_IDInitref(PyInterpreterState *); PyAPI_FUNC(int) _PyInterpreterState_IDIncref(PyInterpreterState *); PyAPI_FUNC(void) _PyInterpreterState_IDDecref(PyInterpreterState *); +PyAPI_FUNC(int) _PyInterpreterState_IsReady(PyInterpreterState *interp); + PyAPI_FUNC(long) _PyInterpreterState_GetWhence(PyInterpreterState *interp); extern void _PyInterpreterState_SetWhence( PyInterpreterState *interp, diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index fb831bc0de80a3..bb93c279a091cf 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -499,7 +499,7 @@ get_whence(PyInterpreterState *interp) static PyInterpreterState * -resolve_interp(PyObject *idobj, int restricted, const char *op) +resolve_interp(PyObject *idobj, int restricted, int reqready, const char *op) { PyInterpreterState *interp; if (idobj == NULL) { @@ -512,6 +512,18 @@ resolve_interp(PyObject *idobj, int restricted, const char *op) } } + if (reqready && !_PyInterpreterState_IsReady(interp)) { + if (idobj == NULL) { + PyErr_Format(PyExc_InterpreterError, + "cannot %s current interpreter (not ready)", op); + } + else { + PyErr_Format(PyExc_InterpreterError, + "cannot %s interpreter %R (not ready)", op, idobj); + } + return NULL; + } + if (restricted && get_whence(interp) != _PyInterpreterState_WHENCE_STDLIB) { if (idobj == NULL) { PyErr_Format(PyExc_InterpreterError, @@ -619,6 +631,7 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) _PyErr_ChainExceptions1(exc); return NULL; } + assert(_PyInterpreterState_IsReady(interp)); PyObject *idobj = _PyInterpreterState_GetIDObject(interp); if (idobj == NULL) { @@ -664,7 +677,9 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) } // Look up the interpreter. - PyInterpreterState *interp = resolve_interp(id, restricted, "destroy"); + int reqready = 0; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "destroy"); if (interp == NULL) { return NULL; } @@ -746,6 +761,7 @@ interp_get_current(PyObject *self, PyObject *Py_UNUSED(ignored)) if (interp == NULL) { return NULL; } + assert(_PyInterpreterState_IsReady(interp)); return get_summary(interp); } @@ -759,6 +775,7 @@ static PyObject * interp_get_main(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyInterpreterState *interp = _PyInterpreterState_Main(); + assert(_PyInterpreterState_IsReady(interp)); return get_summary(interp); } @@ -782,8 +799,9 @@ interp_set___main___attrs(PyObject *self, PyObject *args, PyObject *kwargs) } // Look up the interpreter. - PyInterpreterState *interp = resolve_interp(id, restricted, - "update __main__ for"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "update __main__ for"); if (interp == NULL) { return NULL; } @@ -942,7 +960,9 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = resolve_interp(id, restricted, "exec code for"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "exec code for"); if (interp == NULL) { return NULL; } @@ -1004,7 +1024,9 @@ interp_call(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = resolve_interp(id, restricted, "make a call in"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "make a call in"); if (interp == NULL) { return NULL; } @@ -1060,7 +1082,9 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = resolve_interp(id, restricted, "run a string in"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "run a string in"); if (interp == NULL) { return NULL; } @@ -1102,8 +1126,9 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = resolve_interp(id, restricted, - "run a function in"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "run a function in"); if (interp == NULL) { return NULL; } @@ -1172,8 +1197,9 @@ interp_is_running(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = resolve_interp(id, restricted, - "check if running for"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "check if running for"); if (interp == NULL) { return NULL; } @@ -1206,8 +1232,9 @@ interp_get_config(PyObject *self, PyObject *args, PyObject *kwds) idobj = NULL; } - PyInterpreterState *interp = resolve_interp(idobj, restricted, - "get the config of"); + int reqready = 0; + PyInterpreterState *interp = \ + resolve_interp(idobj, restricted, reqready, "get the config of"); if (interp == NULL) { return NULL; } @@ -1272,7 +1299,9 @@ interp_incref(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = resolve_interp(id, restricted, "incref"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "incref"); if (interp == NULL) { return NULL; } @@ -1299,7 +1328,9 @@ interp_decref(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - PyInterpreterState *interp = resolve_interp(id, restricted, "decref"); + int reqready = 1; + PyInterpreterState *interp = \ + resolve_interp(id, restricted, reqready, "decref"); if (interp == NULL) { return NULL; } diff --git a/Python/crossinterp.c b/Python/crossinterp.c index b77cbf29a6818c..47f9e63730125b 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -1896,7 +1896,7 @@ _PyXI_EndInterpreter(PyInterpreterState *interp, long whence = _PyInterpreterState_GetWhence(interp); assert(whence != _PyInterpreterState_WHENCE_RUNTIME); if (whence == _PyInterpreterState_WHENCE_UNKNOWN) { - assert(!interp->_ready); + assert(!_PyInterpreterState_IsReady(interp)); PyThreadState *tstate = PyThreadState_New(interp); save_tstate = PyThreadState_Swap(tstate); _PyInterpreterState_Clear(tstate); diff --git a/Python/pystate.c b/Python/pystate.c index b0fb210bb5f5f6..acec905484c21f 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1111,6 +1111,13 @@ _PyInterpreterState_ReinitRunningMain(PyThreadState *tstate) // accessors //---------- +int +_PyInterpreterState_IsReady(PyInterpreterState *interp) +{ + return interp->_ready; +} + + static inline int check_interpreter_whence(long whence) { From 4f92794abde4324bdcbb2f30cf02384ae610156e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Apr 2024 17:11:40 -0600 Subject: [PATCH 16/18] Do not expose not-ready interpreters in the high-level module. --- Lib/test/support/interpreters/__init__.py | 2 +- Modules/_xxsubinterpretersmodule.c | 40 ++++++++++++++--------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index 18256a0b8265aa..0a5a9259479be4 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -80,7 +80,7 @@ def create(): def list_all(): """Return all existing interpreters.""" return [Interpreter(id, _whence=whence) - for id, whence in _interpreters.list_all()] + for id, whence in _interpreters.list_all(require_ready=True)] def get_current(): diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index bb93c279a091cf..8fcd4fc4154882 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -719,8 +719,17 @@ So does an unrecognized ID."); static PyObject * -interp_list_all(PyObject *self, PyObject *Py_UNUSED(ignored)) +interp_list_all(PyObject *self, PyObject *args, PyObject *kwargs) { + static char *kwlist[] = {"require_ready", NULL}; + int reqready = 0; + if (!PyArg_ParseTupleAndKeywords(args, kwargs, + "|$p:" MODULE_NAME_STR ".list_all", + kwlist, &reqready)) + { + return NULL; + } + PyObject *ids = PyList_New(0); if (ids == NULL) { return NULL; @@ -728,20 +737,21 @@ interp_list_all(PyObject *self, PyObject *Py_UNUSED(ignored)) PyInterpreterState *interp = PyInterpreterState_Head(); while (interp != NULL) { - PyObject *item = get_summary(interp); - if (item == NULL) { - Py_DECREF(ids); - return NULL; - } + if (!reqready || _PyInterpreterState_IsReady(interp)) { + PyObject *item = get_summary(interp); + if (item == NULL) { + Py_DECREF(ids); + return NULL; + } - // insert at front of list - int res = PyList_Insert(ids, 0, item); - Py_DECREF(item); - if (res < 0) { - Py_DECREF(ids); - return NULL; + // insert at front of list + int res = PyList_Insert(ids, 0, item); + Py_DECREF(item); + if (res < 0) { + Py_DECREF(ids); + return NULL; + } } - interp = PyInterpreterState_Next(interp); } @@ -1417,8 +1427,8 @@ static PyMethodDef module_functions[] = { METH_VARARGS | METH_KEYWORDS, create_doc}, {"destroy", _PyCFunction_CAST(interp_destroy), METH_VARARGS | METH_KEYWORDS, destroy_doc}, - {"list_all", interp_list_all, - METH_NOARGS, list_all_doc}, + {"list_all", _PyCFunction_CAST(interp_list_all), + METH_VARARGS | METH_KEYWORDS, list_all_doc}, {"get_current", interp_get_current, METH_NOARGS, get_current_doc}, {"get_main", interp_get_main, From e38f141d5653bafc9fae65a170d81400a3a1f61e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Apr 2024 17:35:39 -0600 Subject: [PATCH 17/18] Fix destroying a not-ready interpreter. --- Lib/test/test_interpreters/test_api.py | 2 +- Python/crossinterp.c | 27 ++++++++++++++------------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index dd59f5df9a37be..3e3150d614eae9 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -1494,7 +1494,7 @@ def test_whence(self): self.assertEqual(whence, _interpreters.WHENCE_STDLIB) for orig, name in { - # XXX Also check WHENCE_UNKNOWN. + _interpreters.WHENCE_UNKNOWN: 'not ready', _interpreters.WHENCE_LEGACY_CAPI: 'legacy C-API', _interpreters.WHENCE_CAPI: 'C-API', _interpreters.WHENCE_XI: 'cross-interpreter C-API', diff --git a/Python/crossinterp.c b/Python/crossinterp.c index 47f9e63730125b..56aac5bf50ce15 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -1872,6 +1872,20 @@ void _PyXI_EndInterpreter(PyInterpreterState *interp, PyThreadState *tstate, PyThreadState **p_save_tstate) { + long whence = _PyInterpreterState_GetWhence(interp); + assert(whence != _PyInterpreterState_WHENCE_RUNTIME); + + if (!_PyInterpreterState_IsReady(interp)) { + assert(whence == _PyInterpreterState_WHENCE_UNKNOWN); + // PyInterpreterState_Clear() requires the GIL, + // which a not-ready does not have, so we don't clear it. + // That means there may be leaks here until clearing the + // interpreter is fixed. + PyInterpreterState_Delete(interp); + return; + } + assert(whence != _PyInterpreterState_WHENCE_UNKNOWN); + PyThreadState *save_tstate = NULL; PyThreadState *cur_tstate = PyThreadState_GET(); if (tstate == NULL) { @@ -1893,18 +1907,7 @@ _PyXI_EndInterpreter(PyInterpreterState *interp, } } - long whence = _PyInterpreterState_GetWhence(interp); - assert(whence != _PyInterpreterState_WHENCE_RUNTIME); - if (whence == _PyInterpreterState_WHENCE_UNKNOWN) { - assert(!_PyInterpreterState_IsReady(interp)); - PyThreadState *tstate = PyThreadState_New(interp); - save_tstate = PyThreadState_Swap(tstate); - _PyInterpreterState_Clear(tstate); - PyInterpreterState_Delete(interp); - } - else { - Py_EndInterpreter(tstate); - } + Py_EndInterpreter(tstate); if (p_save_tstate != NULL) { save_tstate = *p_save_tstate; From 2274d6ef7956de5b71381d000365f27c58f01fb8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 11 Apr 2024 16:54:16 -0600 Subject: [PATCH 18/18] Hide a variable on non-debug builds. --- Python/crossinterp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Python/crossinterp.c b/Python/crossinterp.c index 56aac5bf50ce15..367e29d40d895a 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -1872,7 +1872,9 @@ void _PyXI_EndInterpreter(PyInterpreterState *interp, PyThreadState *tstate, PyThreadState **p_save_tstate) { +#ifndef NDEBUG long whence = _PyInterpreterState_GetWhence(interp); +#endif assert(whence != _PyInterpreterState_WHENCE_RUNTIME); if (!_PyInterpreterState_IsReady(interp)) {