Skip to content

Commit ed60cd1

Browse files
committed
fix(_xxhash.c): always allocate lock at init for Python 3.9-3.12
The previous lazy allocation in XXHASH_LOCK_MAYBE_INIT only created a PyThread_type_lock if update() was called with data >= 64KB. This meant that if a thread-safe hash object was only used with small data (or never called update()), digest(), copy(), reset(), etc. all ran without any synchronization because XXHASH_LOCK_ACQUIRE was a no-op when lock == NULL. Fix: always allocate the lock at initialization via XXHASH_LOCK_INIT, matching hashlib's approach of always-initialized mutex (HASHLIB_INIT_MUTEX). This eliminates the security window entirely. Changes: - XXHASH_LOCK_INIT now calls PyThread_allocate_lock() instead of setting NULL - XXHASH_LOCK_MAYBE_INIT reduced to a no-op (lock always allocated) - Simplified _do_update by removing the lazy init call and dead 'no lock' branch - locking is now unconditional (no-op in non-lock build) - NULL guards in acquire/release/fini retained as defensive safety Python 3.13+ (PyMutex path) was not affected as it was already always-on.
1 parent 1b4664d commit ed60cd1

1 file changed

Lines changed: 22 additions & 44 deletions

File tree

src/_xxhash.c

Lines changed: 22 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -45,23 +45,18 @@
4545
# define XXHASH_LOCK_ACQUIRE(o) PyMutex_Lock(&(o)->mutex)
4646
# define XXHASH_LOCK_ACQUIRE_BLOCKING(o) XXHASH_LOCK_ACQUIRE(o)
4747
# define XXHASH_LOCK_RELEASE(o) PyMutex_Unlock(&(o)->mutex)
48-
# else /* Python 3.9-3.12: PyThread_type_lock */
48+
# else /* Python 3.9-3.12: PyThread_type_lock (always-on, no lazy init) */
4949
# define XXHASH_LOCK_FIELD PyThread_type_lock lock;
50-
# define XXHASH_LOCK_INIT(o) ((o)->lock = NULL)
50+
# define XXHASH_LOCK_INIT(o) ((o)->lock = PyThread_allocate_lock())
51+
/* Always-on: lock is allocated in __init__, matching hashlib's approach.
52+
* NULL is only possible if memory allocation fails; guards are defensive. */
5153
# define XXHASH_LOCK_IS_ACTIVE(o) ((o)->lock != NULL)
52-
/* Lazy allocation on first large update */
53-
# define XXHASH_LOCK_MAYBE_INIT(o, len) \
54-
do { \
55-
if ((o)->lock == NULL && (len) >= XXHASH_GIL_MINSIZE) { \
56-
(o)->lock = PyThread_allocate_lock(); \
57-
/* fail? lock stays NULL, fall back to non-threaded code. */ \
58-
} \
59-
} while (0)
54+
/* Lazy init removed: lock is always allocated. Safety check retained. */
55+
# define XXHASH_LOCK_MAYBE_INIT(o, len) ((void)0)
6056
# define XXHASH_LOCK_FINI(o) do { if ((o)->lock) \
6157
PyThread_free_lock((o)->lock); \
6258
} while (0)
63-
/* Acquire lock when GIL is already released — simple blocking acquire.
64-
* Only acquires if lock has been allocated (lazy init). */
59+
/* Acquire lock when GIL is already released — simple blocking acquire. */
6560
# define XXHASH_LOCK_ACQUIRE_BLOCKING(o) \
6661
do { \
6762
if ((o)->lock) { \
@@ -70,13 +65,12 @@
7065
} while (0)
7166

7267
/* Acquire lock with the GIL held — non-blocking try first, then release
73-
* GIL and block if contested (matches hashlib's ENTER_HASHLIB in 3.9-3.12).
74-
* Only acquires if lock has been allocated (lazy init). */
68+
* GIL and block if contested (matches hashlib's ENTER_HASHLIB in 3.9-3.12). */
7569
# define XXHASH_LOCK_ACQUIRE(o) \
7670
do { \
7771
if ((o)->lock) { \
7872
if (!PyThread_acquire_lock((o)->lock, NOWAIT_LOCK)) { \
79-
/* Lock contested release GIL while waiting. */ \
73+
/* Lock contested release GIL while waiting. */ \
8074
Py_BEGIN_ALLOW_THREADS \
8175
PyThread_acquire_lock((o)->lock, WAIT_LOCK); \
8276
Py_END_ALLOW_THREADS \
@@ -641,40 +635,24 @@ static void PYXXH32_dealloc(PYXXH32Object *self)
641635
}
642636

643637
/* Macro to generate _do_update for each hash type.
644-
* When XXHASH_WITH_LOCK is defined: matches CPython 3.9-3.12 md5 pattern,
645-
* release GIL first (for large data), then acquire lock, hash, release lock,
646-
* re-acquire GIL. For small data, acquire lock with GIL held
647-
* (try-then-block if contested).
648-
* When XXHASH_WITH_LOCK is not defined: no locking, but still release GIL
649-
* for large data to avoid blocking other threads. */
638+
* Lock is always acquired (no-op in non-lock build). For large data,
639+
* release GIL while blocking on lock to avoid stalling other threads. */
650640
#define XXHASH_DO_UPDATE(type, update_fn) \
651641
static inline void \
652642
PY##type##_do_update(PY##type##Object *self, Py_buffer *buf) \
653643
{ \
654-
XXHASH_LOCK_MAYBE_INIT(self, buf->len); \
655-
if (XXHASH_LOCK_IS_ACTIVE(self)) { \
656-
if (buf->len > XXHASH_GIL_MINSIZE) { \
657-
/* Release GIL first, then acquire lock. */ \
658-
Py_BEGIN_ALLOW_THREADS \
659-
XXHASH_LOCK_ACQUIRE_BLOCKING(self); \
660-
update_fn(self->xxhash_state, buf->buf, buf->len); \
661-
XXHASH_LOCK_RELEASE(self); \
662-
Py_END_ALLOW_THREADS \
663-
} else { \
664-
/* Acquire lock with GIL held. */ \
665-
XXHASH_LOCK_ACQUIRE(self); \
666-
update_fn(self->xxhash_state, buf->buf, buf->len); \
667-
XXHASH_LOCK_RELEASE(self); \
668-
} \
644+
if (buf->len > XXHASH_GIL_MINSIZE) { \
645+
/* Release GIL first, then acquire lock. */ \
646+
Py_BEGIN_ALLOW_THREADS \
647+
XXHASH_LOCK_ACQUIRE_BLOCKING(self); \
648+
update_fn(self->xxhash_state, buf->buf, buf->len); \
649+
XXHASH_LOCK_RELEASE(self); \
650+
Py_END_ALLOW_THREADS \
669651
} else { \
670-
/* No lock */ \
671-
if (buf->len > XXHASH_GIL_MINSIZE) { \
672-
Py_BEGIN_ALLOW_THREADS \
673-
update_fn(self->xxhash_state, buf->buf, buf->len); \
674-
Py_END_ALLOW_THREADS \
675-
} else { \
676-
update_fn(self->xxhash_state, buf->buf, buf->len); \
677-
} \
652+
/* Acquire lock with GIL held (try-then-block if contested). */ \
653+
XXHASH_LOCK_ACQUIRE(self); \
654+
update_fn(self->xxhash_state, buf->buf, buf->len); \
655+
XXHASH_LOCK_RELEASE(self); \
678656
} \
679657
PyBuffer_Release(buf); \
680658
}

0 commit comments

Comments
 (0)