bpo-35381 Make all posixmodule types heap-allocated by eduardo-elizondo · Pull Request #10854 · python/cpython (original) (raw)

I have made the requested changes; please review again

cc @vstinner @ericsnowcurrently @serhiy-storchaka

Hi Serihy. Thanks for pointing out the bug, I've updated the change with the appropriate fix.
I was able to come up with a generic solution which can solve many of the bugs that have come up from converting a PyType_Ready to a PyType_FromSpec. I used that to solve the specific case that is presented in posixmodule.c

Problem

Most of the issues in converting a PyType_Ready to a PyType_FromSpec come from implicit assumptions about how these types should behave. This results in missing signals which cause incorrect behavior. For example, uninstantiable types becoming instantiable, the runtime crashing, or refcnt leaks. To understand these issues I have broken down the scenarios that might arise:

tp_new is defined and tp_dealloc is defined.

Result: Refcnt Leak or Crash

Instances are most likely instantiated in Python-land and it uses PyType_GenericAlloc to allocate the instance, increfing the type. However, under the assumption that the type is static, it is likely that tp_dealloc is not decrefing the type. This results in a net refcnt of +1 per object instantiation.
Another sub-scenario is when tp_dealloc actually decrefs the type. This results in correct behavior.
However, if the user creates the instance through PyObject_New{Var} it won't incref its type. This results in a net refcnt of -1 per object instantiation, causing a a segfault.

tp_new is defined and tp_dealloc is undefined.

Result: Correct Behavior

Instances are most likely instantiated in Python-land and it uses PyType_GenericAlloc to allocate the instance, increfing the type. tp_dealloc is undefined and will inherit subtype_dealloc which will decref the type. This results in correct behavior.
However, if the user creates the instance through PyObject_New{Var} it won't incref its type. This results in a net refcnt of -1 per object instantiation, causing a a segfault.

tp_new is undefined and tp_dealloc is defined. Result:

Incorrect Behavior

This is an uninstantiable type from Python-land. Therefore, instantiation comes directly from PyObject_New{Var}, which won't incref the type. Therefore, the defined tp_dealloc won't decref the type either. However, the side effect is that the signal produced by tp_new being NULL will be lost given that the type will inherit its base object's tp_new. This will now make the type both instantiable and pickable in Python-land.

tp_new is undefined and tp_dealloc is unedefined.

Result: Crash

This is an uninstantiable type from Python-land. Therefore, instantiation comes directly from PyObject_New{Var}, which won't incref the type. However, the undefined tp_dealloc will inherit subtype_dealloc which will decref the type. This results in a net refcnt of -1 per object instantiation, causing a a segfault.

Solution

In order to solve these issues, we need to think of these types as participating in normal refcnting just like any other instance.

The following provides a generic solution for all these different scenarios:

• Make PyObject_INIT incref heap-allocated instance's type.
• Have the custom tp_dealloc decref the instance's type
• Explicitly define a tp_new, __reduce__, and __reduce_ex__ that raise an exception and return NULL to make it uninstantiable and unpickable respectively.

Closing Bugs

The generic solution proposed here should be able to address all the issues and problems in the following bpos:

https://bugs.python.org/issue14936
https://bugs.python.org/issue15142
https://bugs.python.org/issue15721
https://bugs.python.org/issue16690
https://bugs.python.org/issue23815
https://bugs.python.org/issue26979