New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PEP 3121, 384 refactoring applied to hashopenssl module #59858
Comments
+ m = PyState_FindModule(&_hashlibmodule); Why is this dance needed? + if((void *)type->tp_dealloc == (void *)EVP_dealloc) { Why? |
Something else: +#define _hashlibstate(o) ((_hashlibstate *)PyModule_GetState(o)) It is really bad style to #define a symbol that shadows another symbol. |
Regarding the macro definition, I would be fine with changing it to _hashlib_state. The dance you have found inside the Init, makes shure that the very same module is returned if Init is called twice or multiple times, before the Module is unloaded. A month back, when I created this patch, I had statements such as test.import.import_fresh_module(...) call the Init-method multiple times, before a module was unloaded. This was apparently a bug, as I can no longer reproduce this behavior, but at that time I thought it was the expected behavior :-) The last code snipped verifies, that we only dereference the type if the dealloc function is not being called from inside the subtype_dealloc function. This is necessary because the subtype_dealloc function itself contains a decref of the respective type object. Without this check, we would then end up decrefing the type too many times. |
I still don't understand why it is required. You shouldn't have to |
Well, as I have changed the static type to a HeapType (as I am now using the stable ABI from PEP-384 for this type), we have to start perfoming proper reference counting with this object. This includes increfing the type in case a new object of that type is created, and decrefing if such an object is being deallocated. |
Le mardi 14 août 2012 à 19:55 +0000, Robin Schreiber a écrit :
No, but see http://bugs.python.org/issue15142 Also, all this is not documented at all :-( |
As subtype_dealloc decref'ed the HeapType I thought the dealloc method was the most appropriate place to decrement the refcount of the type. |
Le mardi 14 août 2012 à 20:16 +0000, Robin Schreiber a écrit :
Yes, of course.
That's a good question. Perhaps the DECREF mechanism / typeobject.c |
Antoine: Py_DECREF calls tp_dealloc directly, so the type needs to be DECREFed in the course of tp_dealloc. I don't think there is any alternative to that. One may wonder why regular extension types don't do that: this is because of a hack that excludes static (non-heap) types from being reference counted in their instances. Heap types do refcount their types, consequently, subtype_dealloc also DECREFs the type. I certainly agree that this is muddy, in particular when it comes to subtyping where the derived subtype calls the base tp_dealloc. In an ideal world, object_dealloc would decref the type, and subtypes would be required to call the base type's dealloc. However, I feel that this cannot be changed before Python 4. So I'd propose that it is actually the leaf subtype which decrefs ob_type. The check whether you are the leaf type is then done by checking whether tp_dealloc is the one you are "in" right now. |
By leaf, you mean the derived subtype? That sounds a bit quirky (you Speaking of which, what does this refactoring bring exactly? The type |
I think this fails (currently) because a subtype defined in Python using
It's really about the PEP-3121 changes: modules shouldn't have any In addition, it further reduces the differences between "extension |
commit 46f59eb
See bpo-41111 "Convert a few stdlib extensions to the limited C API (PEP-384)". |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: