gh-135239: smarter use of mutex in _md5
#135267
Conversation
| static void | ||
| md5_update_state_cond_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) | ||
| { | ||
| ENTER_HASHLIB(self); // conditionally acquire a lock |
There was a problem hiding this comment.
I might be misunderstanding this, but this will never lock on the GIL-ful build, and always lock on free-threading? If that's the case, why not just wrap the lock with a Py_GIL_DISABLED instead of a use_mutex field?
If it's not like that, and you can indeed arbitarily modify use_mutex at runtime, then this isn't thread-safe: use_mutex might get concurrently reassigned, which will lead to deadlocks and worse.
There was a problem hiding this comment.
If it's not like that, and you can indeed arbitarily modify use_mutex at runtime,
Not arbitrarily but it's possible to set use_mutex to true if we give a large data to hash.
There was a problem hiding this comment.
But let me actually think about it tomorrow. I feel that I actually messed up my conditions somewhere.
There was a problem hiding this comment.
I think a general good rule of thumb is that conditional locking is evil.
I've taken the liberty of normalizing code style. I'll do the same in other modules, (SHA and BLAKE2). That way, I'll never need to touch cosmetics again in crypto-modules. Well, if it's too much I can drop the last commit.