Uh oh!
There was an error while loading. Please reload this page.
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.
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
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: Victor Stinner <vstinner@python.org>
corona10
commented
Mar 30, 2020
@vstinner Updated! Please take a look |
vstinner
left a comment
There was a problem hiding this comment.
LGTM. Sorry, just one last change request, I promise ;-)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
vstinner
commented
Mar 30, 2020
Thanks, I merged your PR. |
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