From d96358ff9de646dbf64dfdfed46d510da7ec4803 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Thu, 25 Jan 2024 22:46:32 +0300 Subject: gh-114315: Make `threading.Lock` a real class, not a factory function (#114479) `threading.Lock` is now the underlying class and is constructable rather than the old factory function. This allows for type annotations to refer to it which had no non-ugly way to be expressed prior to this. --------- Co-authored-by: Alex Waygood Co-authored-by: Gregory P. Smith --- Doc/library/threading.rst | 7 +++-- Lib/test/test_threading.py | 20 +++++++++---- Lib/threading.py | 4 +-- .../2024-01-23-14-11-49.gh-issue-114315.KeVdzl.rst | 2 ++ Modules/_threadmodule.c | 33 +++++++++++++++++++--- 5 files changed, 52 insertions(+), 14 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-01-23-14-11-49.gh-issue-114315.KeVdzl.rst diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst index b85b7f0..5fbf937 100644 --- a/Doc/library/threading.rst +++ b/Doc/library/threading.rst @@ -534,9 +534,10 @@ All methods are executed atomically. lock, subsequent attempts to acquire it block, until it is released; any thread may release it. - Note that ``Lock`` is actually a factory function which returns an instance - of the most efficient version of the concrete Lock class that is supported - by the platform. + .. versionchanged:: 3.13 + ``Lock`` is now a class. In earlier Pythons, ``Lock`` was a factory + function which returned an instance of the underlying private lock + type. .. method:: acquire(blocking=True, timeout=-1) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index dbdc46f..1ab223b 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -171,11 +171,21 @@ class ThreadTests(BaseTestCase): t.start() t.join() - @cpython_only - def test_disallow_instantiation(self): - # Ensure that the type disallows instantiation (bpo-43916) - lock = threading.Lock() - test.support.check_disallow_instantiation(self, type(lock)) + def test_lock_no_args(self): + threading.Lock() # works + self.assertRaises(TypeError, threading.Lock, 1) + self.assertRaises(TypeError, threading.Lock, a=1) + self.assertRaises(TypeError, threading.Lock, 1, 2, a=1, b=2) + + def test_lock_no_subclass(self): + # Intentionally disallow subclasses of threading.Lock because they have + # never been allowed, so why start now just because the type is public? + with self.assertRaises(TypeError): + class MyLock(threading.Lock): pass + + def test_lock_or_none(self): + import types + self.assertIsInstance(threading.Lock | None, types.UnionType) # Create a bunch of threads, let each do some work, wait until all are # done. diff --git a/Lib/threading.py b/Lib/threading.py index ecf799b..00b95f8 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -5,7 +5,6 @@ import sys as _sys import _thread import functools import warnings -import _weakref from time import monotonic as _time from _weakrefset import WeakSet @@ -37,6 +36,7 @@ __all__ = ['get_ident', 'active_count', 'Condition', 'current_thread', _start_joinable_thread = _thread.start_joinable_thread _daemon_threads_allowed = _thread.daemon_threads_allowed _allocate_lock = _thread.allocate_lock +_LockType = _thread.LockType _set_sentinel = _thread._set_sentinel get_ident = _thread.get_ident _is_main_interpreter = _thread._is_main_interpreter @@ -115,7 +115,7 @@ def gettrace(): # Synchronization classes -Lock = _allocate_lock +Lock = _LockType def RLock(*args, **kwargs): """Factory function that returns a new reentrant lock. diff --git a/Misc/NEWS.d/next/Library/2024-01-23-14-11-49.gh-issue-114315.KeVdzl.rst b/Misc/NEWS.d/next/Library/2024-01-23-14-11-49.gh-issue-114315.KeVdzl.rst new file mode 100644 index 0000000..a8a19fc --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-01-23-14-11-49.gh-issue-114315.KeVdzl.rst @@ -0,0 +1,2 @@ +Make :class:`threading.Lock` a real class, not a factory function. Add +``__new__`` to ``_thread.lock`` type. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 99f97eb..5cceb84 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -5,6 +5,7 @@ #include "Python.h" #include "pycore_interp.h" // _PyInterpreterState.threads.count #include "pycore_moduleobject.h" // _PyModule_GetState() +#include "pycore_modsupport.h" // _PyArg_NoKeywords() #include "pycore_pylifecycle.h" #include "pycore_pystate.h" // _PyThreadState_SetCurrent() #include "pycore_sysmodule.h" // _PySys_GetAttr() @@ -349,6 +350,27 @@ lock__at_fork_reinit(lockobject *self, PyObject *Py_UNUSED(args)) } #endif /* HAVE_FORK */ +static lockobject *newlockobject(PyObject *module); + +static PyObject * +lock_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) +{ + // convert to AC? + if (!_PyArg_NoKeywords("lock", kwargs)) { + goto error; + } + if (!_PyArg_CheckPositional("lock", PyTuple_GET_SIZE(args), 0, 0)) { + goto error; + } + + PyObject *module = PyType_GetModuleByDef(type, &thread_module); + assert(module != NULL); + return (PyObject *)newlockobject(module); + +error: + return NULL; +} + static PyMethodDef lock_methods[] = { {"acquire_lock", _PyCFunction_CAST(lock_PyThread_acquire_lock), @@ -398,6 +420,7 @@ static PyType_Slot lock_type_slots[] = { {Py_tp_methods, lock_methods}, {Py_tp_traverse, lock_traverse}, {Py_tp_members, lock_type_members}, + {Py_tp_new, lock_new}, {0, 0} }; @@ -405,7 +428,7 @@ static PyType_Spec lock_type_spec = { .name = "_thread.lock", .basicsize = sizeof(lockobject), .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | - Py_TPFLAGS_DISALLOW_INSTANTIATION | Py_TPFLAGS_IMMUTABLETYPE), + Py_TPFLAGS_IMMUTABLETYPE), .slots = lock_type_slots, }; @@ -1442,8 +1465,6 @@ A subthread can use this function to interrupt the main thread.\n\ Note: the default signal handler for SIGINT raises ``KeyboardInterrupt``." ); -static lockobject *newlockobject(PyObject *module); - static PyObject * thread_PyThread_allocate_lock(PyObject *module, PyObject *Py_UNUSED(ignored)) { @@ -1841,10 +1862,14 @@ thread_module_exec(PyObject *module) } // Lock - state->lock_type = (PyTypeObject *)PyType_FromSpec(&lock_type_spec); + state->lock_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, &lock_type_spec, NULL); if (state->lock_type == NULL) { return -1; } + if (PyModule_AddType(module, state->lock_type) < 0) { + return -1; + } + // Old alias: lock -> LockType if (PyDict_SetItemString(d, "LockType", (PyObject *)state->lock_type) < 0) { return -1; } -- cgit v0.12