Sorry, something went wrong.
|
@msprotz - does this make sense to you? |
Sorry, something went wrong.
|
Do you have a reference for how the Python GC works? I'm reading https://wiki.python.org/moin/GlobalInterpreterLock, https://docs.python.org/3/c-api/init.html#thread-state-and-the-global-interpreter-lock, and https://docs.python.org/3/c-api/memory.html -- is there any other good resource? Basically I'd like to understand if:
I'd like to read up a little bit before giving you a thumbs-up. Thanks! |
Sorry, something went wrong.
|
CPython is purely references counted with no compaction or moving of objects in memory. The GC exists solely to deal with reference cycles. It never rearranges memory and never frees memory behind any object with a non-zero reference count. We've got a writeup on that at https://devguide.python.org/internals/garbage-collector/. It has been this way since Python 2.0 when the cyclic GC was introduced. (before that, reference cycles were memory leaks) No memory returned from Python C APIs will ever be moved or freed so long as it belongs to referenced objects. The PyBytes objects the hash functions receive always have positive refcounts by definition, thus the PyBytes_AsStringAndSize() returned pointer is safely passed synchronously to other C code with GIL released as our thread owns that immutable object. This PR applies identical logic to what _hashopenssl has done for a very long time to release the GIL. The lock per hash object is added to avoid code being able to call into the C hash state mutation APIs on a given instance from multiple threads at once. (It'd be clearly buggy code design if anyone ever did - our goal is just to avoid undefined behavior of C API misuse should anyone ever try) |
Sorry, something went wrong.
There was a problem hiding this comment.
Ok that makes a lock more sense and now I understand the purpose of the lock. Thanks for the pointers!
Overall I think it would be helpful to document the intent behind this locking behavior, notably:
Other than that, as far as I can tell, this looks fine. Thanks!
Sorry, something went wrong.
|
Agreed, I'll work on some common code comments to apply to everywhere relevant about these patterns. |
Sorry, something went wrong.
|
Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Sorry, something went wrong.
|
GH-104776 is a backport of this pull request to the 3.12 branch. |
Sorry, something went wrong.
This matches the GIL releasing behavior of our existing _hashopenssl
module, extending it to the HACL* built-ins.