Non-blocking _PyMutex_LockTimed spins and may fail unnecessarily in no-GIL builds

Bug report

Bug description:

Non-blocking `_PyMutex_LockTimed` spins and may fail unnecessarily (no-GIL build)

Branch: main (commit 2793b68f758c10fb63b264787f10d46a71fc8086)
Build: configured with --disable-gil (MAX_SPIN_COUNT > 0)
OS: all

Summary

_PyMutex_LockTimed() is supposed to return immediately when called with
timeout == 0 (non-blocking). In a no-GIL build it:

Spins for up to MAX_SPIN_COUNT yields before returning;
Can still return PY_LOCK_FAILURE even though the lock was released during that spin.

Timed/blocking calls (timeout > 0) also waste CPU because the spin loop
never reloads the lock word, so they can’t notice an unlock until after the
maximum spin count.

The bug is invisible in GIL builds because MAX_SPIN_COUNT is 0.

Root cause

/* Python/lock.c — excerpts */

if ((v & _Py_LOCKED) == 0) {
    if (_Py_atomic_compare_exchange_uint8(&m->_bits, &v, v | _Py_LOCKED))
        return PY_LOCK_ACQUIRED;
}
else if (timeout == 0) {            // executes only if FIRST load saw LOCKED
    return PY_LOCK_FAILURE;         // non-blocking, OK
}

/* … later … */
if (!(v & _Py_HAS_PARKED) && spin_count < MAX_SPIN_COUNT) {
    _Py_yield();
    spin_count++;
    continue;                       // BUG: never refreshes v
}

If our fast CAS loses a race, the else if guard is skipped, so a
non-blocking call drops into the spin loop.
Inside that loop v is never reloaded, so the thread can’t see that the
lock became free. After MAX_SPIN_COUNT iterations it falls through to
the same timeout == 0 guard and fails spuriously.

Proposed fix

/* 1 — reload v each spin */
if (!(v & _Py_HAS_PARKED) && spin_count < MAX_SPIN_COUNT) {
    _Py_yield();
    spin_count++;
    v = _Py_atomic_load_uint8_relaxed(&m->_bits);   // ← added
    continue;
}

/* 2 — early-out for timeout == 0 */
if ((v & _Py_LOCKED) == 0) {
    if (_Py_atomic_compare_exchange_uint8(&m->_bits, &v, v | _Py_LOCKED))
        return PY_LOCK_ACQUIRED;
}
if (timeout == 0) {                               // ← moved outside else
    return PY_LOCK_FAILURE;
}

Result

Non-blocking calls now return immediately: success if the CAS wins,
failure if it loses – no spinning, no parking.
Timed/blocking calls still spin for fairness, but they now reload the
lock word each iteration, so they acquire promptly once the lock is free.

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

pythongh-135871: Fix needless spinning in _PyMutex_LockTimed (timeout…

@ZeroIntensity

cc @ZeroIntensity

The issue and proposed fix make sense to me, but I'm also a little suspicious. I would think that this continue needs to reload the lock bits, because otherwise we're going to spin without reloading, as stated in the issue. But maybe this was omitted for contention reasons? I can't find anything in the PR about it.

Sam based PyMutex off of WebKit's locking implementation. Reading the article, it seems that they do include an atomic load for each spin, but it was also written several years before PyMutex went in. There's a chance they removed that atomic for performance's sake in-between 2016 and 2023. I'll wait for his input.

@colesbury

cc @colesbury @mpage

[3.14] gh-135871: Fix needless spinning in `_PyMutex_LockTimed` with …

[3.13] gh-135871: Fix needless spinning in `_PyMutex_LockTimed` with …

I tracked down WebKit's locking implementation, and it seems they do have the atomic load for each iteration: https://github.com/WebKit/WebKit/blob/7ebf83c48305f8aac9f37af71af77dd66b758811/Source/WTF/wtf/LockAlgorithmInlines.h#L48

We found that it hurts performance for us, though. Is everyone good with closing this?

No, we should fix it, but carefully in a way that doesn't hurt performance.

Ok. Perhaps only reloading every few spins or so?

I'm interested into taking this issue if no one objects, and I've started to look into it.

@dpdani

Go ahead @dpdani!

pythongh-135871: Reload lock internal state while spinning in `_PyMut…

gh-135871: Reload lock internal state while spinning in `PyMutex_Lock…

Merge branch 'fix-pythongh-138577' of github.com:CuriousLearner/cpyth…

Fixed by #146064

pythongh-135871: Reload lock internal state while spinning in `PyMute…

gh-157914: Revert "gh-135871: Reload lock internal state while spinni…

[3.15] gh-157914: Revert "gh-135871: Reload lock internal state while…

Navigation Menu

Repository navigation

Non-blocking `_PyMutex_LockTimed` spins and may fail unnecessarily in no-GIL builds #135871

Bug report

Bug description:

Non-blocking `_PyMutex_LockTimed` spins and may fail unnecessarily (no-GIL build)

Summary

Root cause

Proposed fix

Result

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Non-blocking _PyMutex_LockTimed spins and may fail unnecessarily in no-GIL builds #135871

Description

Bug report

Bug description:

Non-blocking _PyMutex_LockTimed spins and may fail unnecessarily (no-GIL build)

Summary

Root cause

Proposed fix

Result

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

picnixz commented on Jun 24, 2025

ZeroIntensity commented on Jun 24, 2025

vstinner commented on Jun 24, 2025

ZeroIntensity commented on Jun 30, 2025

colesbury commented on Jun 30, 2025

ZeroIntensity commented on Jun 30, 2025

dpdani commented on Feb 27, 2026

vstinner commented on Mar 2, 2026

kumaraditya303 commented on Apr 5, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Non-blocking `_PyMutex_LockTimed` spins and may fail unnecessarily in no-GIL builds #135871

Non-blocking `_PyMutex_LockTimed` spins and may fail unnecessarily (no-GIL build)