Skip to content

Commit f4dee84

Browse files
committed
fix: restore try-then-block for PyThread_type_lock to prevent deadlock under GIL
4443f28 incorrectly simplified the 3.9-3.12 PyThread_type_lock path to unconditional WAIT_LOCK. This is unsafe under GIL: PyThread_acquire_lock(lock, WAIT_LOCK) does NOT release the GIL while waiting, unlike PyMutex_Lock (3.13+) which passes _PY_LOCK_DETACH and releases GIL when parking. Deadlock scenario: 1. T1 holds object lock (large data path, GIL released) 2. T2 holds GIL, enters small data path, calls WAIT_LOCK → blocks with GIL 3. T1 finishes hash, tries to reacquire GIL → blocked 4. T2 waits for object lock, T1 waits for GIL → deadlock Fix: restore the try-then-block in XXHASH_LOCK_ACQUIRE (used under GIL): - First try NOWAIT_LOCK (fast path) - If contested, release GIL (Py_BEGIN_ALLOW_THREADS), then WAIT_LOCK - Reacquire GIL (Py_END_ALLOW_THREADS) XXHASH_LOCK_ACQUIRE_BLOCKING (used without GIL, large data path) keeps simple WAIT_LOCK since GIL is already released. 3.13+ PyMutex path remains unconditional (safe due to _PY_LOCK_DETACH).
1 parent 4443f28 commit f4dee84

1 file changed

Lines changed: 37 additions & 9 deletions

File tree

src/_xxhash.c

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,24 +41,52 @@
4141
# define XXHASH_LOCK_INIT(o) ((void)((o)->mutex = (PyMutex){0}))
4242
# define XXHASH_LOCK_FINI(o) ((void)0)
4343
# define XXHASH_LOCK_ACQUIRE(o) PyMutex_Lock(&(o)->mutex)
44+
# define XXHASH_LOCK_ACQUIRE_BLOCKING(o) XXHASH_LOCK_ACQUIRE(o)
4445
# define XXHASH_LOCK_RELEASE(o) PyMutex_Unlock(&(o)->mutex)
4546
# else /* Python 3.9-3.12: PyThread_type_lock (always-on) */
4647
# define XXHASH_LOCK_FIELD PyThread_type_lock lock;
4748
# define XXHASH_LOCK_INIT(o) ((o)->lock = PyThread_allocate_lock())
4849
# define XXHASH_LOCK_FINI(o) do { if ((o)->lock) \
4950
PyThread_free_lock((o)->lock); \
5051
} while (0)
51-
/* Unconditional blocking acquire, matching 3.15's HASHLIB_ACQUIRE_LOCK.
52-
* Under GIL: if lock contended, the holder has released GIL (large data path),
53-
* so we block here without risking deadlock. */
54-
# define XXHASH_LOCK_ACQUIRE(o) PyThread_acquire_lock((o)->lock, WAIT_LOCK)
55-
# define XXHASH_LOCK_RELEASE(o) PyThread_release_lock((o)->lock)
52+
/* Acquire lock when GIL is already released — simple blocking acquire. */
53+
# define XXHASH_LOCK_ACQUIRE_BLOCKING(o) \
54+
do { \
55+
if ((o)->lock) { \
56+
PyThread_acquire_lock((o)->lock, WAIT_LOCK); \
57+
} \
58+
} while (0)
59+
60+
/* Acquire lock with the GIL held — non-blocking try first, then release
61+
* GIL and block if contested (matches hashlib's ENTER_HASHLIB in 3.9-3.12).
62+
* WAIT_LOCK under GIL would deadlock: if another thread holds the object lock
63+
* (large data path, GIL released), this thread blocks with GIL held, preventing
64+
* the holder from re-acquiring GIL to release the lock. */
65+
# define XXHASH_LOCK_ACQUIRE(o) \
66+
do { \
67+
if ((o)->lock) { \
68+
if (!PyThread_acquire_lock((o)->lock, NOWAIT_LOCK)) { \
69+
/* Lock contested — release GIL while waiting. */ \
70+
Py_BEGIN_ALLOW_THREADS \
71+
PyThread_acquire_lock((o)->lock, WAIT_LOCK); \
72+
Py_END_ALLOW_THREADS \
73+
} \
74+
} \
75+
} while (0)
76+
77+
# define XXHASH_LOCK_RELEASE(o) \
78+
do { \
79+
if ((o)->lock) { \
80+
PyThread_release_lock((o)->lock); \
81+
} \
82+
} while (0)
5683
# endif
57-
#else /* !XXHASH_WITH_LOCK */
84+
# else /* !XXHASH_WITH_LOCK */
5885
# define XXHASH_LOCK_FIELD
5986
# define XXHASH_LOCK_INIT(o) ((void)0)
6087
# define XXHASH_LOCK_FINI(o) ((void)0)
6188
# define XXHASH_LOCK_ACQUIRE(o) ((void)0)
89+
# define XXHASH_LOCK_ACQUIRE_BLOCKING(o) ((void)0)
6290
# define XXHASH_LOCK_RELEASE(o) ((void)0)
6391
#endif
6492

@@ -608,14 +636,14 @@ static inline void \
608636
PY##type##_do_update(PY##type##Object *self, Py_buffer *buf) \
609637
{ \
610638
if (buf->len > XXHASH_GIL_MINSIZE) { \
611-
/* Release GIL first, then acquire lock. */ \
639+
/* Release GIL first, then acquire lock (blocking, no GIL). */ \
612640
Py_BEGIN_ALLOW_THREADS \
613-
XXHASH_LOCK_ACQUIRE(self); \
641+
XXHASH_LOCK_ACQUIRE_BLOCKING(self); \
614642
update_fn(self->xxhash_state, buf->buf, buf->len); \
615643
XXHASH_LOCK_RELEASE(self); \
616644
Py_END_ALLOW_THREADS \
617645
} else { \
618-
/* Acquire lock with GIL held. */ \
646+
/* Acquire lock with GIL held (try-then-block to avoid deadlock). */ \
619647
XXHASH_LOCK_ACQUIRE(self); \
620648
update_fn(self->xxhash_state, buf->buf, buf->len); \
621649
XXHASH_LOCK_RELEASE(self); \

0 commit comments

Comments
 (0)