bpo-40077: Convert _abc module to use PyType_FromSpec() #19202
Conversation
Replace statically allocated types with heap allocated types: use PyType_FromSpec(). Add a module state to store the _abc_data_type.. Add traverse, clear and free functions to the module.
| negative cache to be cleared before its next use. | ||
| Note: this counter is private. Use `abc.get_cache_token()` for | ||
| external code. */ | ||
| static unsigned long long abc_invalidation_counter = 0; |
There was a problem hiding this comment.
I am not sure that the abc_invalidation_counter is okay to be shared as static from interpreter Isolation point of view.
There was a problem hiding this comment.
Should we move this into PyInterpreterState?
There was a problem hiding this comment.
It seems like it would be safe to put it in the module state. The purpose of the counter is to invalidate the module cache. Registering a class in module instance 1 should not invalidate module instance 2. What do you think?
There was a problem hiding this comment.
I agree, I 've updated the PR.
Lib/test/test_abc.py
Outdated
There was a problem hiding this comment.
This test might be what we wanted exactly.
I just updated the test for more validation.
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
It seems like you cannot access to the module state in abc_data_new() which prevents you to move abc_invalidation_counter into the module state :-(
I suggest to leave abc_invalidation_counter as a "static" variable, but please add a FIXME like:
/* FIXME: PEP 573: Move abc_invalidation_counter into _abcmodule_state */
It's safe to keep it as a static variable until each interpreter gets its own GIL.
Until it happens, the change should move the module closer to the goal of isolated interpreters.
Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst
Outdated
Show resolved
Hide resolved
Co-Authored-By: Victor Stinner <[email protected]>
|
@vstinner Updated! Please take a look |
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM. Sorry, just one last change request, I promise ;-)
|
Thanks, I merged your PR. |
4 participants
Replace statically allocated types with heap allocated types:
use PyType_FromSpec().
Add a module state to store the _abc_data_type..
Add traverse, clear and free functions to the module.
https://bugs.python.org/issue40077