TypeError for threading.Lock | None

[Mark90]

Bug report

Bug description:

I tried to search for a related ticket by various queries but did not find any, hopefully I haven't overlooked it.

While rewriting a codebase from Optional[foo] to the newer foo | None style, I encountered that threading.Lock does not allow the latter.

Example on MacOS 3.12.1:

>>> from typing import Optional
>>> from threading import Lock
>>> Optional[Lock]
typing.Optional[<built-in function allocate_lock>]
>>> Lock | None
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for |: 'builtin_function_or_method' and 'NoneType'

Given that the documentation states the following, the error makes sense:

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.

I actually didn't know it was a factory function. I would not intentionally create a <function> | None type. But since the documentation describes threading.Lock as a class, one would expect to be able to use it as any other type.

CPython versions tested on:

3.11, 3.12, 3.13.0a3

Operating systems tested on:

Linux, macOS

Linked PRs

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

[sobolevn]

Here are my 2c:

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.

This does not seem to be the case anymore, right now Lock always calls _thread.allocate_lock which is a C-function: thread_PyThread_allocate_lock

Even the doc above says the same thing:

In Python, it is currently the lowest level synchronization primitive available, implemented directly by the _thread extension module.

This rather simple patch fixes the problem:

diff --git Lib/threading.py Lib/threading.py
index 85aff589680..075d7b0dce1 100644
--- Lib/threading.py
+++ Lib/threading.py
@@ -5,7 +5,6 @@
 import _thread
 import functools
 import warnings
-import _weakref
 
 from time import monotonic as _time
 from _weakrefset import WeakSet
@@ -108,7 +107,7 @@ def gettrace():
 
 # Synchronization classes
 
-Lock = _allocate_lock
+Lock = _thread.LockType
 
 def RLock(*args, **kwargs):
     """Factory function that returns a new reentrant lock.
diff --git Modules/_threadmodule.c Modules/_threadmodule.c
index afcf646e3bc..4037e254d22 100644
--- Modules/_threadmodule.c
+++ Modules/_threadmodule.c
@@ -349,6 +349,18 @@ 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)
+{
+    PyObject *module = PyType_GetModuleByDef(type, &thread_module);
+    if (module == NULL) {
+        return NULL;
+    }
+    return (PyObject *)newlockobject(module);
+}
+
 
 static PyMethodDef lock_methods[] = {
     {"acquire_lock", _PyCFunction_CAST(lock_PyThread_acquire_lock),
@@ -398,6 +410,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 +418,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,
 };
 
@@ -1439,8 +1452,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))
 {
@@ -1838,10 +1849,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 (PyDict_SetItemString(d, "LockType", (PyObject *)state->lock_type) < 0) {
         return -1;
     }

It might need some other changes, like restricting the extra arguments. But, generally it should be fine.

Demo:

>>> import threading
>>> threading.Lock()
<unlocked _thread.lock object at 0x1012899a0>
>>> threading.Lock | None
_thread.lock | None

What do others think? @gpshead @vstinner

[vstinner]

IMO it's reasonable to allow creating instances of _thread.LockType by adding a tp_new member and changing its flags.

     .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
-              Py_TPFLAGS_DISALLOW_INSTANTIATION | Py_TPFLAGS_IMMUTABLETYPE),
+              Py_TPFLAGS_IMMUTABLETYPE),

Should we allow subclassing or not? Does it make sense? Does it work?

I don't know the rationale for only having a _thread.allocate_lock() API, rather than exposing directly the _thread.Lock type and allow to create instances, in the threading module.

cc @serhiy-storchaka @pitrou

[gpshead]

_thread (aka thread originally) is based on very old code... I doubt there is any good reason left, if there ever was (even looking back to 2.6 I see no reason for it to be done this way). Changing it to be a constructable type from the C extension instead of a function constructed thing makes sense to me.

Lets not allow subclassing. Because we never have before.

[sobolevn]

I will make a PR soon! 👍

[gpshead]

Fixed in 3.13. Thanks for the discussion & PR. Changing the type of a thing in a stdlib module is not something we can backport as a bugfix unfortunately so people who actually want to annotate something as being a Lock will need to come up with a workaround or just not annotate that.

Ideally APIs aren't passing locks around so I hope the need is rare.

(the question of why threading.Lock wasn't a type and how to annotate for it ironically came up from someone else unrelated to this issue or PR at work this morning)

[AlexWaygood]

FWIW, in typeshed we already pretend that Lock is a class, even though it isn't at runtime: https://github.com/python/typeshed/blob/b4f60ac5bfcab92e5d9666979b68dc0babc4502c/stdlib/threading.pyi#L103-L111

That means that type checkers will accept using threading.Lock in type annotations, even though it isn't really a class (since type checkers don't know anything about the truth of things at runtime, they only know what typeshed tells them).

So in the meantime, you should be able to simply do this if you need to annoate something as being a threading.Lock object:

foo: threading.Lock

And you should be able to do this to solve the original problem the issue author was reporting:

foo: "threading.Lock | None"

(It's obviously not ideal to have to put quotes around annotations, so I'm glad this will be fixed properly in Python 3.13!)