gh-111916: Make hashlib related modules thread-safe without the GIL

[tomasr8]

Still a work in progress but I'd be happy for feedback ;)

I'm not entirely sure what to do about HASHLIB_GIL_MINSIZE. IIUC the original code avoids releasing the GIL and getting a separate lock when the update buffer is short enough. I'm guessing that in the nogil version, we'll have to lock the mutex regardless of the length (i.e. essentially getting rid of the macro)? Is that correct?

As a side note, I'm failing to build this PR with the GIL enabled. Running ./configure --with-pydebug and make -s -j2, I'm getting this error:

Infileincludedfrom ./Modules/hashlib.h:7, from ./Modules/md5module.c:29: ./Include/internal/pycore_lock.h: Infunction ‘PyMutex_LockFast’: ./Include/internal/pycore_lock.h:60:12: error: implicitdeclarationoffunction ‘_Py_atomic_compare_exchange_uint8’; didyoumean ‘__atomic_compare_exchange_n’? [-Werror=implicit-function-declaration] 60 | return_Py_atomic_compare_exchange_uint8(lock_bits, &expected, _Py_LOCKED);

Even if I add #include "pyatomic.h" to pycore_lock.h I get the same error.. any idea why that is?

• Issue: Make hashlib related modules thread-safe without the GIL #111916

Fixes#111916

[colesbury]

The build errors you're seeing are due to md5module.c being compiled with the limited API, which means internal-only functions like PyMutex_Lock are not available. You should remove these lines from md5module.c:

#ifndef _MSC_VER #include "pyconfig.h" // Py_NOGIL #endif #ifndef Py_NOGIL // Need limited C API version 3.12 for Py_MOD_PER_INTERPRETER_GIL_SUPPORTED #define Py_LIMITED_API 0x030c0000 #endif 

And instead add:

#ifndef Py_BUILD_CORE_BUILTIN # define Py_BUILD_CORE_MODULE 1 #endif 

[colesbury]

(I edited my review comment to note that you also should add Py_BUILD_CORE_MODULE to md5module.c)

[colesbury]

It looks like some of the generated parts of Modules/_blake2/blake2s_impl.c are out of date. You'll need to re-run Argument Clinic. I think python3 Tools/clinic/clinic.py --make --exclude Lib/test/clinic.test.c --srcdir . should do the trick.

[tomasr8]

Thanks for the thorough review! I reran AC as you suggested, so hopefully it should be good this time

[colesbury]

Thanks, @tomasr8! This LGTM.

@gpshead - would you be able to review this PR? (I saw your name on the git log for these files.)

[gpshead]

one minor style nit, overall looks great. thanks!

[gpshead]

the original code avoids releasing the GIL and getting a separate lock when the update buffer is short enough. I'm guessing that in the nogil version, we'll have to lock the mutex regardless of the length

Yep, the old HASHLIB_GIL_MINSIZE logic doesn't make sense in a free threaded build - always locking as this PR does works there. This PR could probably elide the concept from the code entirely during a free-threaded build, but that'd just makes the maintenance harder with additional ifdefs. For little value. It'll be an always-taken branch because use_mutex = true in free-threaded builds so the size will never get checked. Good enough - it won't have a measurable performance impact.