Skip to content

gh-114315: Make threading.Lock a real class, not a factory function #114479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions Doc/library/threading.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 15 additions & 5 deletions Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,21 @@ def test_args_argument(self):
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.
Expand Down
4 changes: 2 additions & 2 deletions Lib/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import _thread
import functools
import warnings
import _weakref

from time import monotonic as _time
from _weakrefset import WeakSet
Expand Down Expand Up @@ -37,6 +36,7 @@
_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
Expand Down Expand Up @@ -108,7 +108,7 @@ def gettrace():

# Synchronization classes

Lock = _allocate_lock
Lock = _LockType

def RLock(*args, **kwargs):
"""Factory function that returns a new reentrant lock.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Make :class:`threading.Lock` a real class, not a factory function. Add
``__new__`` to ``_thread.lock`` type.
33 changes: 29 additions & 4 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -398,14 +420,15 @@ 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}
};

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,
};

Expand Down Expand Up @@ -1439,8 +1462,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))
{
Expand Down Expand Up @@ -1838,10 +1859,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;
}
Expand Down