diff options
author | Sam Gross <colesbury@gmail.com> | 2024-07-08 14:52:07 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-08 14:52:07 -0400 |
commit | 1d3cf79a501a93a7a488fc75d4db3060c5ee7d1a (patch) | |
tree | 2e2f59a0d466ee4abccae054281be060c578e094 /Python | |
parent | 31873bea471020ca5deaf735d9acb0f1abeb1d3c (diff) | |
download | cpython-1d3cf79a501a93a7a488fc75d4db3060c5ee7d1a.tar.gz cpython-1d3cf79a501a93a7a488fc75d4db3060c5ee7d1a.zip |
gh-121368: Fix seq lock memory ordering in _PyType_Lookup (#121388)
The `_PySeqLock_EndRead` function needs an acquire fence to ensure that
the load of the sequence happens after any loads within the read side
critical section. The missing fence can trigger bugs on macOS arm64.
Additionally, we need a release fence in `_PySeqLock_LockWrite` to
ensure that the sequence update is visible before any modifications to
the cache entry.
Diffstat (limited to 'Python')
-rw-r--r-- | Python/lock.c | 26 |
1 files changed, 15 insertions, 11 deletions
diff --git a/Python/lock.c b/Python/lock.c index 7c6a5175e88..57675fe1873 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -514,6 +514,7 @@ void _PySeqLock_LockWrite(_PySeqLock *seqlock) } else if (_Py_atomic_compare_exchange_uint32(&seqlock->sequence, &prev, prev + 1)) { // We've locked the cache + _Py_atomic_fence_release(); break; } else { @@ -547,28 +548,31 @@ uint32_t _PySeqLock_BeginRead(_PySeqLock *seqlock) return sequence; } -uint32_t _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous) +int _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous) { - // Synchronize again and validate that the entry hasn't been updated - // while we were readying the values. - if (_Py_atomic_load_uint32_acquire(&seqlock->sequence) == previous) { + // gh-121368: We need an explicit acquire fence here to ensure that + // this load of the sequence number is not reordered before any loads + // within the read lock. + _Py_atomic_fence_acquire(); + + if (_Py_atomic_load_uint32_relaxed(&seqlock->sequence) == previous) { return 1; - } + } - _Py_yield(); - return 0; + _Py_yield(); + return 0; } -uint32_t _PySeqLock_AfterFork(_PySeqLock *seqlock) +int _PySeqLock_AfterFork(_PySeqLock *seqlock) { // Synchronize again and validate that the entry hasn't been updated // while we were readying the values. - if (SEQLOCK_IS_UPDATING(seqlock->sequence)) { + if (SEQLOCK_IS_UPDATING(seqlock->sequence)) { seqlock->sequence = 0; return 1; - } + } - return 0; + return 0; } #undef PyMutex_Lock |