_thread.lock.release() is not thread-safe in free-threaded builds

Bug report

Bug description:

The implementation of _thread.lock.release() manipulates the locked field in a thread-unsafe way (this may be called by a thread that does not hold the lock) in free-threaded builds:

cpython/Modules/_threadmodule.c

Lines 813 to 825 in 630df37

    
           static PyObject * 
        
           lock_PyThread_release_lock(lockobject *self, PyObject *Py_UNUSED(ignored)) 
        
           { 
        
               /* Sanity check: the lock must be locked */ 
        
               if (!self->locked) { 
        
                   PyErr_SetString(ThreadError, "release unlocked lock"); 
        
                   return NULL; 
        
               } 
        
               self->locked = 0; 
        
               PyThread_release_lock(self->lock_lock); 
        
               Py_RETURN_NONE; 
        
           }

We're choosing to punt on this for 3.13 since this should only be problematic for contended unlocks. We can revisit this for 3.13 if it turns out to be an issue in practice.

Post 3.13 we would like to change the underlying lock to be a PyMutex and replace the implementation with something like:

static PyObject *
lock_PyThread_release_lock(lockobject *self, PyObject *Py_UNUSED(ignored))
{
    /* Sanity check: the lock must be locked */
    if (_PyMutex_TryUnlock(&self->mutex) < 0) {
        PyErr_SetString(ThreadError, "release unlocked lock");
        return NULL;
    }

    Py_RETURN_NONE;
}

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs

Hi,
I have been trying a free-threaded build, I really enjoy the new possibilities, thank you so much for your work!

I am currently working on implementing part of the OpenMP API in native Python, and I need to be able to prevent concurrent updates to shared variables (If you are familiar with OpenMP, implementing the critical construct, allowing only one thread to run a region at a time).

Note that this is custom syntax I'm introducing with my library, I'm giving this extract to give an idea of what I'm talking about. Here is the example I would like to run.

@omp.enable
def main():
    acc = 0
    with OpenMP("parallel"):
        with OpenMP("for"):
            for i in range(1000):
                with OpenMP("critical"):
                    acc += i
    print(acc) # Actual output
    print(sum(range(1000))) # Expected output

I decided to use a threading.Lock instance to protect the region, however it seems like there are concurrent accesses despite the protection. I keep getting results that are different from what I get with the GIL, or a deadlock.

An example of output I get in python3.14t

497975 # Actual output
499500 # Expected output

But really most of the time I get a deadlock when all I'm doing is using a shared threading.Lock instance in a context manager to protect the acc += i

Is there any way to protect shared variables from concurrent accesses in user code written in pure Python?

This should only be problematic for contended unlocks.

I didn't check the details of the race conditions in the implementation _thread.lock, but I imagine from what you say that the behaviour I'm facing is highlighted by the fact the region I'm trying to protect is a single instruction. In any case,
protecting these updates are to me a fundamental aspect of multithreaded programming and HPC.

Thanks,
Dorian.

It sounds like you have some other bug in your code. threading.Lock will protect a region from concurrent access. Make sure you are actually locking the same lock, and don't have something like threads locking different lock instances.

The issue described by Matt is only relevant about the errors raised when trying to unlock a lock that is already unlocked (i.e., an error raised when misusing the lock)

Thank you for the clarification, I am using a context manager so I don't think I would be unlocking an already unlocked lock.

Sorry to have bothered you, have a great day!

Thanks,
Dorian.

gh-117721: use PyMutex in `_thread.lock` (#125110)

pythongh-117721: use PyMutex in `_thread.lock` (python#125110)

[3.13] gh-117721: use PyMutex in `_thread.lock` (#125110) (#125116)

pythongh-117721: use PyMutex in `_thread.lock` (python#125110)

Navigation Menu

`_thread.lock.release()` is not thread-safe in free-threaded builds #117721

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	static PyObject *
	lock_PyThread_release_lock(lockobject self, PyObject Py_UNUSED(ignored))
	{
	/* Sanity check: the lock must be locked */
	if (!self->locked) {
	PyErr_SetString(ThreadError, "release unlocked lock");
	return NULL;
	}

	self->locked = 0;
	PyThread_release_lock(self->lock_lock);
	Py_RETURN_NONE;
	}

_thread.lock.release() is not thread-safe in free-threaded builds #117721

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

douakli commented on Jul 27, 2024

colesbury commented on Jul 27, 2024

douakli commented on Jul 28, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

`_thread.lock.release()` is not thread-safe in free-threaded builds #117721