gh-114271: Make _thread.lock thread-safe in free-threaded builds (#116433)

mpage · web-flow · commit c62144a02cfa · 2024-03-06T15:46:36.000-05:00
Previously, the `locked` field was set after releasing the lock. This reverses
the order so that the `locked` field is set while the lock is still held.

There is still one thread-safety issue where `locked` is checked prior to
releasing the lock, however, in practice that will only be an issue when
unlocking the lock is contended, which should be rare.
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
@@ -390,8 +390,8 @@ lock_PyThread_release_lock(lockobject *self, PyObject *Py_UNUSED(ignored))
         return NULL;
     }
 
-    PyThread_release_lock(self->lock_lock);
     self->locked = 0;
+    PyThread_release_lock(self->lock_lock);
     Py_RETURN_NONE;
 }
 
@@ -1665,8 +1665,8 @@ release_sentinel(void *weakref_raw)
     lockobject *lock = (lockobject *)_PyWeakref_GET_REF(weakref);
     if (lock != NULL) {
         if (lock->locked) {
-            PyThread_release_lock(lock->lock_lock);
             lock->locked = 0;
+            PyThread_release_lock(lock->lock_lock);
         }
         Py_DECREF(lock);
     }

Original file line number	Diff line number	Diff line change
`@@ -390,8 +390,8 @@ lock_PyThread_release_lock(lockobject self, PyObject Py_UNUSED(ignored))`
`390`	`390`	`return NULL;`
`391`	`391`	`}`
`392`	`392`
`393`		`- PyThread_release_lock(self->lock_lock);`
`394`	`393`	`self->locked = 0;`
	`394`	`+ PyThread_release_lock(self->lock_lock);`
`395`	`395`	`Py_RETURN_NONE;`
`396`	`396`	`}`
`397`	`397`
`@@ -1665,8 +1665,8 @@ release_sentinel(void *weakref_raw)`
`1665`	`1665`	`lockobject lock = (lockobject )_PyWeakref_GET_REF(weakref);`
`1666`	`1666`	`if (lock != NULL) {`
`1667`	`1667`	`if (lock->locked) {`
`1668`		`- PyThread_release_lock(lock->lock_lock);`
`1669`	`1668`	`lock->locked = 0;`
	`1669`	`+ PyThread_release_lock(lock->lock_lock);`
`1670`	`1670`	`}`
`1671`	`1671`	`Py_DECREF(lock);`
`1672`	`1672`	`}`