From 2c3fbf5e654271e893ad44b95b1e3abb4f7e8fef Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Fri, 17 Jan 2025 23:41:48 +0100 Subject: [PATCH 01/13] Check thread arguments Adds a check to thread constructors ensuring that only cowns, immutables, and externally unique regions can be passed in as thread arguments. --- Lib/test/test_using.py | 12 ++++++++++++ Lib/threading.py | 16 ++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/Lib/test/test_using.py b/Lib/test/test_using.py index ef005b34c4a2a6..eddf136697aacd 100644 --- a/Lib/test/test_using.py +++ b/Lib/test/test_using.py @@ -166,3 +166,15 @@ def _(): result = c.get().value.value if result != 200: self.fail() + + def test_thread_creation(self): + from threading import Thread as T + + class Mutable: pass + self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'args' : (Mutable(),) }) + self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'kwargs' : {'a' : Mutable()} }) + self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'args' : (Mutable(),), 'kwargs' : {'a' : Mutable()} }) + + T(target=print, args=(42, Cown(), Region())) + T(target=print, kwargs={'imm' : 42, 'cown' : Cown(), 'region' : Region()}) + self.assertTrue(True) # To make sure we got here correctly diff --git a/Lib/threading.py b/Lib/threading.py index df273870fa4273..2d77998bfd4fb8 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -895,6 +895,22 @@ class is implemented. except AttributeError: pass + def movable(o): + return isinstance(o, Region) and not o.is_open() + + for k, v in kwargs.items(): + if not (isimmutable(v) or isinstance(v, Cown) or movable(v)): + raise RuntimeError(f'thread was passed {k} : {type(v)} -- ' + 'only immutable objects, cowns and free ' + 'regions may be passed to a thread') + for a in args: + if not (isimmutable(a) or isinstance(a, Cown) or movable(a)): + from sys import getrefcount as rc + print(a, rc(a)) + raise RuntimeError(f'thread was passed {type(a)} -- ' + 'only immutable objects, cowns and free ' + 'regions may be passed to a thread') + self._target = target self._name = name self._args = args From 16afce86c5b026b56d306ed30a990ae234dd91c6 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Sat, 18 Jan 2025 11:32:49 +0100 Subject: [PATCH 02/13] Made pyrona checks conditional on region use --- Lib/threading.py | 32 +++++++++++++++++--------------- Python/bltinmodule.c | 9 +++++++++ Python/clinic/bltinmodule.c.h | 20 ++++++++++++++++++++ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/Lib/threading.py b/Lib/threading.py index 2d77998bfd4fb8..4ec925806143a7 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -895,21 +895,23 @@ class is implemented. except AttributeError: pass - def movable(o): - return isinstance(o, Region) and not o.is_open() - - for k, v in kwargs.items(): - if not (isimmutable(v) or isinstance(v, Cown) or movable(v)): - raise RuntimeError(f'thread was passed {k} : {type(v)} -- ' - 'only immutable objects, cowns and free ' - 'regions may be passed to a thread') - for a in args: - if not (isimmutable(a) or isinstance(a, Cown) or movable(a)): - from sys import getrefcount as rc - print(a, rc(a)) - raise RuntimeError(f'thread was passed {type(a)} -- ' - 'only immutable objects, cowns and free ' - 'regions may be passed to a thread') + # Only check when a program uses pyrona + if is_pyrona_program(): + def movable(o): + return isinstance(o, Region) and not o.is_open() + + for k, v in kwargs.items(): + if not (isimmutable(v) or isinstance(v, Cown) or movable(v)): + raise RuntimeError(f'thread was passed {k} : {type(v)} -- ' + 'only immutable objects, cowns and free ' + 'regions may be passed to a thread') + for a in args: + if not (isimmutable(a) or isinstance(a, Cown) or movable(a)): + from sys import getrefcount as rc + print(a, rc(a)) + raise RuntimeError(f'thread was passed {type(a)} -- ' + 'only immutable objects, cowns and free ' + 'regions may be passed to a thread') self._target = target self._name = name diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 89d51a9d7d3bab..cdd22a125a88f2 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -2776,6 +2776,14 @@ builtin_makeimmutable(PyObject *module, PyObject *obj) return Py_MakeImmutable(obj); } +extern bool invariant_do_region_check; + +static PyObject * +builtin_is_pyrona_program_impl(PyObject *module) +{ + return invariant_do_region_check ? Py_True : Py_False; +} + /*[clinic input] invariant_failure_src as builtin_invariantsrcfailure @@ -3118,6 +3126,7 @@ static PyMethodDef builtin_methods[] = { BUILTIN_MAKEIMMUTABLE_METHODDEF BUILTIN_INVARIANTSRCFAILURE_METHODDEF BUILTIN_INVARIANTTGTFAILURE_METHODDEF + BUILTIN_IS_PYRONA_PROGRAM_METHODDEF BUILTIN_ITER_METHODDEF BUILTIN_AITER_METHODDEF BUILTIN_LEN_METHODDEF diff --git a/Python/clinic/bltinmodule.c.h b/Python/clinic/bltinmodule.c.h index 34d29e1b5c81a2..72a2d4485f0b46 100644 --- a/Python/clinic/bltinmodule.c.h +++ b/Python/clinic/bltinmodule.c.h @@ -1481,4 +1481,24 @@ builtin_enableinvariant(PyObject *module, PyObject *Py_UNUSED(ignored)) { return builtin_enableinvariant_impl(module); } + +PyDoc_STRVAR(builtin_is_pyrona_program__doc__, +"is_pyrona_program($module, /)\n" +"--\n" +"\n" +"Returns True if the program has used regions or cowns."); + +static PyObject * +builtin_is_pyrona_program_impl(PyObject *module); + +static PyObject * +builtin_is_pyrona_program(PyObject *module, PyObject *Py_UNUSED(ignored)) +/*[clinic end generated code: output=4e665122542dfd24 input=bec4cf1797c848d4]*/ +{ + return builtin_is_pyrona_program_impl(module); +} + +#define BUILTIN_IS_PYRONA_PROGRAM_METHODDEF \ + {"is_pyrona_program", (PyCFunction)builtin_is_pyrona_program, METH_NOARGS, builtin_is_pyrona_program__doc__}, + /*[clinic end generated code: output=3a883fa08bbd248e input=a9049054013a1b77]*/ From 05260f0a3ee0a76dcf0815b039d06439fcd278e8 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Sun, 19 Jan 2025 09:18:53 +0100 Subject: [PATCH 03/13] Clinic --- Python/bltinmodule.c | 9 --------- Python/clinic/bltinmodule.c.h | 20 -------------------- 2 files changed, 29 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index cdd22a125a88f2..89d51a9d7d3bab 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -2776,14 +2776,6 @@ builtin_makeimmutable(PyObject *module, PyObject *obj) return Py_MakeImmutable(obj); } -extern bool invariant_do_region_check; - -static PyObject * -builtin_is_pyrona_program_impl(PyObject *module) -{ - return invariant_do_region_check ? Py_True : Py_False; -} - /*[clinic input] invariant_failure_src as builtin_invariantsrcfailure @@ -3126,7 +3118,6 @@ static PyMethodDef builtin_methods[] = { BUILTIN_MAKEIMMUTABLE_METHODDEF BUILTIN_INVARIANTSRCFAILURE_METHODDEF BUILTIN_INVARIANTTGTFAILURE_METHODDEF - BUILTIN_IS_PYRONA_PROGRAM_METHODDEF BUILTIN_ITER_METHODDEF BUILTIN_AITER_METHODDEF BUILTIN_LEN_METHODDEF diff --git a/Python/clinic/bltinmodule.c.h b/Python/clinic/bltinmodule.c.h index 72a2d4485f0b46..34d29e1b5c81a2 100644 --- a/Python/clinic/bltinmodule.c.h +++ b/Python/clinic/bltinmodule.c.h @@ -1481,24 +1481,4 @@ builtin_enableinvariant(PyObject *module, PyObject *Py_UNUSED(ignored)) { return builtin_enableinvariant_impl(module); } - -PyDoc_STRVAR(builtin_is_pyrona_program__doc__, -"is_pyrona_program($module, /)\n" -"--\n" -"\n" -"Returns True if the program has used regions or cowns."); - -static PyObject * -builtin_is_pyrona_program_impl(PyObject *module); - -static PyObject * -builtin_is_pyrona_program(PyObject *module, PyObject *Py_UNUSED(ignored)) -/*[clinic end generated code: output=4e665122542dfd24 input=bec4cf1797c848d4]*/ -{ - return builtin_is_pyrona_program_impl(module); -} - -#define BUILTIN_IS_PYRONA_PROGRAM_METHODDEF \ - {"is_pyrona_program", (PyCFunction)builtin_is_pyrona_program, METH_NOARGS, builtin_is_pyrona_program__doc__}, - /*[clinic end generated code: output=3a883fa08bbd248e input=a9049054013a1b77]*/ From dce65bfab695cf91ea17a523ae48398cf04853b1 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Sun, 19 Jan 2025 09:35:37 +0100 Subject: [PATCH 04/13] Clinic fix --- Include/regions.h | 2 ++ Objects/regions.c | 3 +++ Python/bltinmodule.c | 14 ++++++++++++++ Python/clinic/bltinmodule.c.h | 20 +++++++++++++++++++- 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/Include/regions.h b/Include/regions.h index 52a7ec3e9d15a5..1cfc49ce0984b6 100644 --- a/Include/regions.h +++ b/Include/regions.h @@ -15,6 +15,8 @@ PyAPI_FUNC(int) _Py_IsLocal(PyObject *op); PyAPI_FUNC(int) _Py_IsCown(PyObject *op); #define Py_IsCown(op) _Py_IsCown(_PyObject_CAST(op)) +int Py_is_invariant_enabled(void); + #ifdef __cplusplus } #endif diff --git a/Objects/regions.c b/Objects/regions.c index 7f138973965bd3..f4747cd20349f4 100644 --- a/Objects/regions.c +++ b/Objects/regions.c @@ -50,6 +50,9 @@ static void _PyErr_Region(PyObject *src, PyObject *tgt, const char *msg); * Global status for performing the region check. */ bool invariant_do_region_check = false; +int Py_is_invariant_enabled(void) { + return invariant_do_region_check; +} // The src object for an edge that invalidated the invariant. PyObject* invariant_error_src = Py_None; diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 89d51a9d7d3bab..65fbb57c8808e6 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -2815,6 +2815,19 @@ builtin_enableinvariant_impl(PyObject *module) return Py_EnableInvariant(); } +/*[clinic input] +is_pyrona_program as builtin_is_pyrona_program + +Returns True if the program has used regions or cowns. +[clinic start generated code]*/ + +static PyObject * +builtin_is_pyrona_program_impl(PyObject *module) +/*[clinic end generated code: output=2b6729469da00221 input=d36655a3dc34a881]*/ +{ + return Py_is_invariant_enabled() ? Py_True : Py_False; +} + typedef struct { PyObject_HEAD Py_ssize_t tuplesize; @@ -3116,6 +3129,7 @@ static PyMethodDef builtin_methods[] = { BUILTIN_ISSUBCLASS_METHODDEF BUILTIN_ISIMMUTABLE_METHODDEF BUILTIN_MAKEIMMUTABLE_METHODDEF + BUILTIN_IS_PYRONA_PROGRAM_METHODDEF BUILTIN_INVARIANTSRCFAILURE_METHODDEF BUILTIN_INVARIANTTGTFAILURE_METHODDEF BUILTIN_ITER_METHODDEF diff --git a/Python/clinic/bltinmodule.c.h b/Python/clinic/bltinmodule.c.h index 34d29e1b5c81a2..1771189974cfbf 100644 --- a/Python/clinic/bltinmodule.c.h +++ b/Python/clinic/bltinmodule.c.h @@ -1481,4 +1481,22 @@ builtin_enableinvariant(PyObject *module, PyObject *Py_UNUSED(ignored)) { return builtin_enableinvariant_impl(module); } -/*[clinic end generated code: output=3a883fa08bbd248e input=a9049054013a1b77]*/ + +PyDoc_STRVAR(builtin_is_pyrona_program__doc__, +"is_pyrona_program($module, /)\n" +"--\n" +"\n" +"Returns True if the program has used regions or cowns."); + +#define BUILTIN_IS_PYRONA_PROGRAM_METHODDEF \ + {"is_pyrona_program", (PyCFunction)builtin_is_pyrona_program, METH_NOARGS, builtin_is_pyrona_program__doc__}, + +static PyObject * +builtin_is_pyrona_program_impl(PyObject *module); + +static PyObject * +builtin_is_pyrona_program(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + return builtin_is_pyrona_program_impl(module); +} +/*[clinic end generated code: output=c1f628b019efa501 input=a9049054013a1b77]*/ From f1cc65e0609f4ffc503fc8c98638c9c6a6dd04a1 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 20 Jan 2025 12:35:47 +0100 Subject: [PATCH 05/13] Improved threading check, added technical debt todos --- Lib/threading.py | 50 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/Lib/threading.py b/Lib/threading.py index 4ec925806143a7..7619da1ed5c071 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -896,22 +896,44 @@ class is implemented. pass # Only check when a program uses pyrona + from sys import getrefcount as rc + # TODO: improve this check for final version of phase 3 + # - Revisit the rc checks + # - Consider throwing a different kind of error (e.g. RegionError) + # - Improve error messages if is_pyrona_program(): - def movable(o): - return isinstance(o, Region) and not o.is_open() - - for k, v in kwargs.items(): - if not (isimmutable(v) or isinstance(v, Cown) or movable(v)): - raise RuntimeError(f'thread was passed {k} : {type(v)} -- ' - 'only immutable objects, cowns and free ' - 'regions may be passed to a thread') + def ok_share(o): + if isimmutable(o): + return True + if isinstance(o, Cown): + return True + return False + def ok_move(o): + if isinstance(o, Region): + if rc(o) != 4: + # rc = 4 because: + # 1. ref to o in rc + # 2. ref to o on this frame + # 3. ref to o on the calling frame + # 4. ref to o from kwargs dictionary or args tuple/list + raise RuntimeError("Region passed to thread was not moved into thread") + if o.is_open(): + raise RuntimeError("Region passed to thread was open") + return True + return False + + for k in kwargs: + # rc(args) == 6 because we need to know that the args list is moved into the thread too + # TODO: Why 6??? + v = kwargs[k] + if not (ok_share(v) or (ok_move(v) and rc(kwargs) == 6)): + raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") + for a in args: - if not (isimmutable(a) or isinstance(a, Cown) or movable(a)): - from sys import getrefcount as rc - print(a, rc(a)) - raise RuntimeError(f'thread was passed {type(a)} -- ' - 'only immutable objects, cowns and free ' - 'regions may be passed to a thread') + # rc(args) == 6 because we need to know that the args list is moved into the thread too + # TODO: Why 6??? + if not (ok_share(a) or (ok_move(a) and rc(args) == 6)): + raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") self._target = target self._name = name From c7ce3d8185e4b588f1fb98fb37d12d7b2293d6be Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 20 Jan 2025 12:39:22 +0100 Subject: [PATCH 06/13] Added TODO for builtin_is_pyrona_program_impl --- Python/bltinmodule.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 65fbb57c8808e6..36232e780c198e 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -2825,6 +2825,12 @@ static PyObject * builtin_is_pyrona_program_impl(PyObject *module) /*[clinic end generated code: output=2b6729469da00221 input=d36655a3dc34a881]*/ { + // TODO: This is an abuse of notions. We need to revisit + // the definition of when a program is a Pyrona program + // at some later point. The reason for having the definition + // conflated with the invariant being enabled is to only + // perform Pyrona checks (see threading.py) when a program + // must adhere to these checks for correctness. return Py_is_invariant_enabled() ? Py_True : Py_False; } From 1c949a032602decb2609dc2fccfdd9062c63d5c5 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 20 Jan 2025 14:16:56 +0100 Subject: [PATCH 07/13] Stupid trailing whitespace --- Python/bltinmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 36232e780c198e..f9dc986736d61e 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -2825,10 +2825,10 @@ static PyObject * builtin_is_pyrona_program_impl(PyObject *module) /*[clinic end generated code: output=2b6729469da00221 input=d36655a3dc34a881]*/ { - // TODO: This is an abuse of notions. We need to revisit + // TODO: This is an abuse of notions. We need to revisit // the definition of when a program is a Pyrona program // at some later point. The reason for having the definition - // conflated with the invariant being enabled is to only + // conflated with the invariant being enabled is to only // perform Pyrona checks (see threading.py) when a program // must adhere to these checks for correctness. return Py_is_invariant_enabled() ? Py_True : Py_False; From 7ff62389a57d61680c871ddb1137207d336ff3a4 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 27 Jan 2025 20:44:35 +0100 Subject: [PATCH 08/13] Made separate PyronaThread constructor in using.py --- Lib/test/test_using.py | 2 +- Lib/threading.py | 40 ------------------------------- Lib/using.py | 53 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 41 deletions(-) diff --git a/Lib/test/test_using.py b/Lib/test/test_using.py index eddf136697aacd..086df63dde88df 100644 --- a/Lib/test/test_using.py +++ b/Lib/test/test_using.py @@ -168,7 +168,7 @@ def _(): self.fail() def test_thread_creation(self): - from threading import Thread as T + from threading import PyronaThread as T class Mutable: pass self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'args' : (Mutable(),) }) diff --git a/Lib/threading.py b/Lib/threading.py index 7619da1ed5c071..df273870fa4273 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -895,46 +895,6 @@ class is implemented. except AttributeError: pass - # Only check when a program uses pyrona - from sys import getrefcount as rc - # TODO: improve this check for final version of phase 3 - # - Revisit the rc checks - # - Consider throwing a different kind of error (e.g. RegionError) - # - Improve error messages - if is_pyrona_program(): - def ok_share(o): - if isimmutable(o): - return True - if isinstance(o, Cown): - return True - return False - def ok_move(o): - if isinstance(o, Region): - if rc(o) != 4: - # rc = 4 because: - # 1. ref to o in rc - # 2. ref to o on this frame - # 3. ref to o on the calling frame - # 4. ref to o from kwargs dictionary or args tuple/list - raise RuntimeError("Region passed to thread was not moved into thread") - if o.is_open(): - raise RuntimeError("Region passed to thread was open") - return True - return False - - for k in kwargs: - # rc(args) == 6 because we need to know that the args list is moved into the thread too - # TODO: Why 6??? - v = kwargs[k] - if not (ok_share(v) or (ok_move(v) and rc(kwargs) == 6)): - raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") - - for a in args: - # rc(args) == 6 because we need to know that the args list is moved into the thread too - # TODO: Why 6??? - if not (ok_share(a) or (ok_move(a) and rc(args) == 6)): - raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") - self._target = target self._name = name self._args = args diff --git a/Lib/using.py b/Lib/using.py index 30ae93a3bbb1cf..af8f5d76dac7d7 100644 --- a/Lib/using.py +++ b/Lib/using.py @@ -45,3 +45,56 @@ def decorator(func): with CS(cowns, *args): return func() return decorator + +def PyronaThread(group=None, target=None, name=None, + args=(), kwargs=None, *, daemon=None): + # Only check when a program uses pyrona + from sys import getrefcount as rc + from threading import Thread + # TODO: improve this check for final version of phase 3 + # - Revisit the rc checks + # - Consider throwing a different kind of error (e.g. RegionError) + # - Improve error messages + def ok_share(o): + if isimmutable(o): + return True + if isinstance(o, Cown): + return True + return False + def ok_move(o): + if isinstance(o, Region): + if rc(o) != 4: + # rc = 4 because: + # 1. ref to o in rc + # 2. ref to o on this frame + # 3. ref to o on the calling frame + # 4. ref to o from kwargs dictionary or args tuple/list + raise RuntimeError("Region passed to thread was not moved into thread") + if o.is_open(): + raise RuntimeError("Region passed to thread was open") + return True + return False + + if kwargs is None: + for a in args: + # rc(args) == 3 because we need to know that the args list is moved into the thread too + # rc = 3 because: + # 1. ref to args in rc + # 2. ref to args on this frame + # 3. ref to args on the calling frame + if not (ok_share(a) or (ok_move(a) and rc(args) == 3)): + raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") + return Thread(group, target, name, args, daemon) + else: + for k in kwargs: + # rc(args) == 3 because we need to know that keyword dict is moved into the thread too + # rc = 3 because: + # 1. ref to kwargs in rc + # 2. ref to kwargs on this frame + # 3. ref to kwargs on the calling frame + v = kwargs[k] + if not (ok_share(v) or (ok_move(v) and rc(kwargs) == 3)): + raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") + return Thread(group, target, name, kwargs, daemon) + + From 250fa9769c5d066cf5a05fa7b268fa2b5b4c459c Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Mon, 27 Jan 2025 20:50:18 +0100 Subject: [PATCH 09/13] Added comment suggested by @mjp41 and fixed trailing space bug --- Lib/test/test_using.py | 2 +- Lib/using.py | 5 +++-- Objects/regions.c | 3 +++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_using.py b/Lib/test/test_using.py index 086df63dde88df..e9fbbb8b7c4235 100644 --- a/Lib/test/test_using.py +++ b/Lib/test/test_using.py @@ -166,7 +166,7 @@ def _(): result = c.get().value.value if result != 200: self.fail() - + def test_thread_creation(self): from threading import PyronaThread as T diff --git a/Lib/using.py b/Lib/using.py index af8f5d76dac7d7..e0501368bd900b 100644 --- a/Lib/using.py +++ b/Lib/using.py @@ -46,6 +46,9 @@ def decorator(func): return func() return decorator +# TODO: this creates a normal Python thread and ensures that all its +# arguments are moved to the new thread. Eventually we should revisit +# this behaviour as we go multiple interpreters / multicore. def PyronaThread(group=None, target=None, name=None, args=(), kwargs=None, *, daemon=None): # Only check when a program uses pyrona @@ -96,5 +99,3 @@ def ok_move(o): if not (ok_share(v) or (ok_move(v) and rc(kwargs) == 3)): raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") return Thread(group, target, name, kwargs, daemon) - - diff --git a/Objects/regions.c b/Objects/regions.c index f4747cd20349f4..6cf0ad06a6814e 100644 --- a/Objects/regions.c +++ b/Objects/regions.c @@ -50,6 +50,9 @@ static void _PyErr_Region(PyObject *src, PyObject *tgt, const char *msg); * Global status for performing the region check. */ bool invariant_do_region_check = false; +/** + * TODO: revisit the definition of this builting function + */ int Py_is_invariant_enabled(void) { return invariant_do_region_check; } From 627bdeac5e0207d20e9c831565cb368cc5677ea5 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Tue, 28 Jan 2025 18:41:01 +0100 Subject: [PATCH 10/13] Fixed import bug --- Lib/test/test_using.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_using.py b/Lib/test/test_using.py index e9fbbb8b7c4235..f6285a9a7a68ed 100644 --- a/Lib/test/test_using.py +++ b/Lib/test/test_using.py @@ -168,7 +168,7 @@ def _(): self.fail() def test_thread_creation(self): - from threading import PyronaThread as T + from using import PyronaThread as T class Mutable: pass self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'args' : (Mutable(),) }) From e39f73f1738ae28bded1740a417023f5d77d24de Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Sat, 1 Feb 2025 11:39:11 +0100 Subject: [PATCH 11/13] Refactoring of checking logic Co-authored-by: Matthew Parkinson --- Lib/using.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/Lib/using.py b/Lib/using.py index e0501368bd900b..be77a381a88d66 100644 --- a/Lib/using.py +++ b/Lib/using.py @@ -78,24 +78,19 @@ def ok_move(o): return True return False + def check(a, args): + # rc(args) == 3 because we need to know that the args list is moved into the thread too + # rc = 3 because: + # 1. ref to args in rc + # 2. ref to args on this frame + # 3. ref to args on the calling framedef check(a, args): + if not ok_share(a) or (ok_move(a) and rc(args) == 3): + raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") if kwargs is None: for a in args: - # rc(args) == 3 because we need to know that the args list is moved into the thread too - # rc = 3 because: - # 1. ref to args in rc - # 2. ref to args on this frame - # 3. ref to args on the calling frame - if not (ok_share(a) or (ok_move(a) and rc(args) == 3)): - raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") + check(a, args) return Thread(group, target, name, args, daemon) else: for k in kwargs: - # rc(args) == 3 because we need to know that keyword dict is moved into the thread too - # rc = 3 because: - # 1. ref to kwargs in rc - # 2. ref to kwargs on this frame - # 3. ref to kwargs on the calling frame - v = kwargs[k] - if not (ok_share(v) or (ok_move(v) and rc(kwargs) == 3)): - raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") + check(k, kwargs) return Thread(group, target, name, kwargs, daemon) From d408a0f5f4366589643490120bab1be7109743f2 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Sat, 1 Feb 2025 12:43:09 +0100 Subject: [PATCH 12/13] Improve test and update RC due to refactored logic --- Lib/test/test_using.py | 18 ++++++++++++++---- Lib/using.py | 26 ++++++++++++++++---------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_using.py b/Lib/test/test_using.py index f6285a9a7a68ed..8a1fd9a74a7b5b 100644 --- a/Lib/test/test_using.py +++ b/Lib/test/test_using.py @@ -171,10 +171,20 @@ def test_thread_creation(self): from using import PyronaThread as T class Mutable: pass - self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'args' : (Mutable(),) }) - self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'kwargs' : {'a' : Mutable()} }) - self.assertRaises(RuntimeError, T, kwargs = { 'target' : print, 'args' : (Mutable(),), 'kwargs' : {'a' : Mutable()} }) + self.assertRaises(RuntimeError, lambda x: T(target=print, args=(Mutable(),)), None) + self.assertRaises(RuntimeError, lambda x: T(target=print, kwargs={'a' : Mutable()}), None) + self.assertRaises(RuntimeError, lambda x: T(target=print, args=(Mutable(),), kwargs={'a' : Mutable()}), None) + self.assertRaises(RuntimeError, lambda x: T(target=print, args=(Mutable(), 42)), None) + self.assertRaises(RuntimeError, lambda x: T(target=print, args=(Mutable(), Cown())), None) + self.assertRaises(RuntimeError, lambda x: T(target=print, args=(Mutable(), Region())), None) - T(target=print, args=(42, Cown(), Region())) T(target=print, kwargs={'imm' : 42, 'cown' : Cown(), 'region' : Region()}) + T(target=print, kwargs={'a': 42}) + T(target=print, kwargs={'a': Cown()}) + T(target=print, kwargs={'a': Region()}) + + T(target=print, args=(42, Cown(), Region())) + T(target=print, args=(42,)) + T(target=print, args=(Cown(),)) + T(target=print, args=(Region(),)) self.assertTrue(True) # To make sure we got here correctly diff --git a/Lib/using.py b/Lib/using.py index be77a381a88d66..aafc4e30b1e343 100644 --- a/Lib/using.py +++ b/Lib/using.py @@ -49,6 +49,7 @@ def decorator(func): # TODO: this creates a normal Python thread and ensures that all its # arguments are moved to the new thread. Eventually we should revisit # this behaviour as we go multiple interpreters / multicore. +# TODO: require RC to be one less when move is upstreamed def PyronaThread(group=None, target=None, name=None, args=(), kwargs=None, *, daemon=None): # Only check when a program uses pyrona @@ -66,12 +67,13 @@ def ok_share(o): return False def ok_move(o): if isinstance(o, Region): - if rc(o) != 4: + if rc(o) != 5: # rc = 4 because: # 1. ref to o in rc - # 2. ref to o on this frame - # 3. ref to o on the calling frame - # 4. ref to o from kwargs dictionary or args tuple/list + # 2. ref to o on this frame (ok_move) + # 3. ref to o on the calling frame (check) + # 4. ref to o from iteration over kwargs dictionary or args tuple/list + # 5. ref to o from kwargs dictionary or args tuple/list raise RuntimeError("Region passed to thread was not moved into thread") if o.is_open(): raise RuntimeError("Region passed to thread was open") @@ -79,18 +81,22 @@ def ok_move(o): return False def check(a, args): - # rc(args) == 3 because we need to know that the args list is moved into the thread too - # rc = 3 because: + # rc(args) == 4 because we need to know that the args list is moved into the thread too + # rc = 4 because: # 1. ref to args in rc # 2. ref to args on this frame # 3. ref to args on the calling framedef check(a, args): - if not ok_share(a) or (ok_move(a) and rc(args) == 3): + # 4. ref from frame calling PyronaThread -- FIXME: not valid; revisit after #45 + if not (ok_share(a) or (ok_move(a) and rc(args) == 4)): raise RuntimeError("Thread was passed an object which was neither immutable, a cown, or a unique region") + if kwargs is None: for a in args: check(a, args) - return Thread(group, target, name, args, daemon) + return Thread(group, target, name, args, daemon) else: for k in kwargs: - check(k, kwargs) - return Thread(group, target, name, kwargs, daemon) + # Important to get matching RCs in both paths + v = kwargs[k] + check(v, kwargs) + return Thread(group, target, name, kwargs, daemon) From c9ef79d83b9d7e6274140c7cfac27debe4d364f3 Mon Sep 17 00:00:00 2001 From: Tobias Wrigstad Date: Sat, 1 Feb 2025 12:45:56 +0100 Subject: [PATCH 13/13] Removed is_pyrona_program builtin --- Python/bltinmodule.c | 20 -------------------- Python/clinic/bltinmodule.c.h | 20 +------------------- 2 files changed, 1 insertion(+), 39 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index f9dc986736d61e..89d51a9d7d3bab 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -2815,25 +2815,6 @@ builtin_enableinvariant_impl(PyObject *module) return Py_EnableInvariant(); } -/*[clinic input] -is_pyrona_program as builtin_is_pyrona_program - -Returns True if the program has used regions or cowns. -[clinic start generated code]*/ - -static PyObject * -builtin_is_pyrona_program_impl(PyObject *module) -/*[clinic end generated code: output=2b6729469da00221 input=d36655a3dc34a881]*/ -{ - // TODO: This is an abuse of notions. We need to revisit - // the definition of when a program is a Pyrona program - // at some later point. The reason for having the definition - // conflated with the invariant being enabled is to only - // perform Pyrona checks (see threading.py) when a program - // must adhere to these checks for correctness. - return Py_is_invariant_enabled() ? Py_True : Py_False; -} - typedef struct { PyObject_HEAD Py_ssize_t tuplesize; @@ -3135,7 +3116,6 @@ static PyMethodDef builtin_methods[] = { BUILTIN_ISSUBCLASS_METHODDEF BUILTIN_ISIMMUTABLE_METHODDEF BUILTIN_MAKEIMMUTABLE_METHODDEF - BUILTIN_IS_PYRONA_PROGRAM_METHODDEF BUILTIN_INVARIANTSRCFAILURE_METHODDEF BUILTIN_INVARIANTTGTFAILURE_METHODDEF BUILTIN_ITER_METHODDEF diff --git a/Python/clinic/bltinmodule.c.h b/Python/clinic/bltinmodule.c.h index 1771189974cfbf..34d29e1b5c81a2 100644 --- a/Python/clinic/bltinmodule.c.h +++ b/Python/clinic/bltinmodule.c.h @@ -1481,22 +1481,4 @@ builtin_enableinvariant(PyObject *module, PyObject *Py_UNUSED(ignored)) { return builtin_enableinvariant_impl(module); } - -PyDoc_STRVAR(builtin_is_pyrona_program__doc__, -"is_pyrona_program($module, /)\n" -"--\n" -"\n" -"Returns True if the program has used regions or cowns."); - -#define BUILTIN_IS_PYRONA_PROGRAM_METHODDEF \ - {"is_pyrona_program", (PyCFunction)builtin_is_pyrona_program, METH_NOARGS, builtin_is_pyrona_program__doc__}, - -static PyObject * -builtin_is_pyrona_program_impl(PyObject *module); - -static PyObject * -builtin_is_pyrona_program(PyObject *module, PyObject *Py_UNUSED(ignored)) -{ - return builtin_is_pyrona_program_impl(module); -} -/*[clinic end generated code: output=c1f628b019efa501 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=3a883fa08bbd248e input=a9049054013a1b77]*/