TypeError for threading.Lock | None

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

@gpshead

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

@serhiy-storchaka

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

_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.

I will make a PR soon! 👍

pythongh-114315: Make `threading.Lock` a real class

gh-114315: Make `threading.Lock` a real class, not a factory function (…

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)

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!)

pythongh-114315: Make `threading.Lock` a real class, not a factory fu…

Prevent Optional[RLock] to be fixed

fix: wrap threading Timer, Lock in quotes

creating a type union with the pipe operator is not support by thread…

see issue python/cpython#114315

feat: Activate ruff rules UP(pyupgrade) (#3871)

Navigation Menu

Repository navigation

TypeError for threading.Lock | None #114315

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

4 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

TypeError for threading.Lock | None #114315

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

sobolevn commented on Jan 22, 2024

vstinner commented on Jan 22, 2024

gpshead commented on Jan 22, 2024

sobolevn commented on Jan 22, 2024

gpshead commented on Jan 25, 2024

AlexWaygood commented on Jan 25, 2024

4 remaining items

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions