Issue40240
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2020-04-09 15:00 by Eric Cousineau, last changed 2022-04-11 14:59 by admin. This issue is now closed.
| Messages (13) | |||
|---|---|---|---|
| msg366059 - (view) | Author: Eric Cousineau (Eric Cousineau) * | Date: 2020-04-09 15:00 | |
Motivated by this downstream project issue that I am working on: https://github.com/RobotLocomotion/drake/issues/13026 In https://bugs.python.org/issue32377, I encountered PEP 442's updated resurrection behavior when moving from supporting Python 2 to Python 3. There, Antoine Pitrou (pitrou) said that using this API (finalized + set finalized) could work, but that I could also try recreating the wrapper object. I have not yet attempted his suggestion given that (a) wrapping code is nuanced (pybind11, inheritance, etc.) and (b) this API has been working for us for the past 2 years. Related to this, I saw some mentions of breakage of Cython due to its usage of this API: https://bugs.python.org/issue35081#msg330045 The breakage was mitigated by keeping this internal API exposed (so kinda public, but not really?). Is it at all possible to considering making some of this public API? |
|||
| msg366066 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-04-09 15:20 | |
See also bpo-40241: "[C API] Make PyGC_Head structure opaque, or even more it to the internal C API". |
|||
| msg366069 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-04-09 15:27 | |
> Is it at all possible to considering making some of this public API? In bpo-35081, I wanted to move PyGC macros to the internal C API because they are private functions, but also because they expose implementation details. Example: #define _PyGC_PREV_MASK_FINALIZED (1) #define _PyGCHead_FINALIZED(g) \ (((g)->_gc_prev & _PyGC_PREV_MASK_FINALIZED) != 0) #define _Py_AS_GC(o) ((PyGC_Head *)(o)-1) #define _PyGC_FINALIZED(o) \ _PyGCHead_FINALIZED(_Py_AS_GC(o)) _Py_AS_GC(o) emits machine code which hardcodes the size and alignment of the PyGC_Head structure. If PyGC_Head is changed, machine code will crash or misbehave at least. And that happened recently: bpo-27987 changed PyGC_Head between Python 3.7.4 and 3.7.5 with commit 8766cb74e186d3820db0a855ccd780d6d84461f7. I'm not against exposing the "feature" in the public C API. I'm only against exposing macros which "leak" implementation details. What I did recently is to add regular functions in the public C API, and keep macros and static inline functions for the internal C API. We can for example add "int PyGC_Finalized(PyObject *obj);" function which would be opaque in term of ABI. |
|||
| msg366076 - (view) | Author: Eric Cousineau (Eric Cousineau) * | Date: 2020-04-09 16:01 | |
C functions sound great! I am certainly not wed to the macros (nor do I love them), as I do not have intense performance requirements where inlining (and spilling implementation guts) is necessary. |
|||
| msg366077 - (view) | Author: Pablo Galindo Salgado (pablogsal) * ![]() |
Date: 2020-04-09 16:23 | |
There is a specific Python function in 3.9 for it: https://docs.python.org/3.9/library/gc.html#gc.is_finalized So it may sense to add a function to query if an object is finalized, but I am not sure it makes sense to allow to mark an object as finalized because that could mess with the GC algorithm. Certainly exposing the macros per se may be a bad idea because there are too many implementation details there (like where the flags are stored) so at least a new function will be needed. |
|||
| msg366078 - (view) | Author: Pablo Galindo Salgado (pablogsal) * ![]() |
Date: 2020-04-09 16:25 | |
I will make a PR for exposing function to query if an object is finalized as for sure there is value on having that in the public API. |
|||
| msg366086 - (view) | Author: Eric Cousineau (Eric Cousineau) * | Date: 2020-04-09 20:26 | |
> [...] but I am not sure it makes sense to allow to mark an object as finalized because that could mess with the GC algorithm. Actually, I would like the opposite, to mark it unfinalized to resurrect the object more than once. PEP 442 from a ways back had this effect (per bpo-32377), and Antoine updated the docs after I filed the issue. I didn't chime in early enough to snag the "more-than-once" functionality, so I guess this is what I get for dipping into private API without asking to make it public until 2 years later... d'oh! |
|||
| msg366087 - (view) | Author: Eric Cousineau (Eric Cousineau) * | Date: 2020-04-09 20:27 | |
Er, to clarify: "PEP 442 had that effect" => "PEP 442 had the effect of making it resurrectable only once, but not more than that." |
|||
| msg366088 - (view) | Author: Pablo Galindo Salgado (pablogsal) * ![]() |
Date: 2020-04-09 21:06 | |
> Actually, I would like the opposite, to mark it unfinalized to resurrect the object more than once. That is out of contract and goes against the guarantees on the GC and can (will) cause the finalizer of the object to be executed more than once. I don't think is a good idea to expose an API that allows breaking the guarantees that we give: every object is finalized once. Also, taking into account that someone could mark externally objects as not finalized can make things more complicated for us to maintain in the collector. |
|||
| msg366093 - (view) | Author: Eric Cousineau (Eric Cousineau) * | Date: 2020-04-10 00:16 | |
Aye. Using a workaround for now ("leak" the object by incrementing the refcount on first resurrection):
https://github.com/RobotLocomotion/pybind11/pull/39
I may try Antoine's suggestion later on, but will definitely reformulate this to use the public API version of `gc.is_finalized()` when it comes about.
Thanks!
|
|||
| msg366171 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-04-11 00:25 | |
New changeset f13072b8a89a922285737988b086beb4b06c6648 by Pablo Galindo in branch 'master': bpo-40241: Add PyObject_GC_IsTracked and PyObject_GC_IsFinalized to the public C-API (GH-19461) https://github.com/python/cpython/commit/f13072b8a89a922285737988b086beb4b06c6648 |
|||
| msg395870 - (view) | Author: Irit Katriel (iritkatriel) * ![]() |
Date: 2021-06-15 11:38 | |
Can this be closed now? It seems that the query API was added and the set API was rejected. |
|||
| msg395877 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2021-06-15 12:51 | |
Pablo: "That is out of contract and goes against the guarantees on the GC and can (will) cause the finalizer of the object to be executed more than once. I don't think is a good idea to expose an API that allows breaking the guarantees that we give: every object is finalized once." I concur with Pablo. We must not expose an API to "unfinalize" an object. Irit Katriel: "Can this be closed now? It seems that the query API was added and the set API was rejected." Right. I close the issue as rejected. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:29 | admin | set | github: 84421 |
| 2021-06-15 12:51:26 | vstinner | set | status: open -> closed resolution: rejected messages: + msg395877 stage: resolved |
| 2021-06-15 11:38:48 | iritkatriel | set | nosy:
+ iritkatriel messages: + msg395870 |
| 2020-04-11 00:25:22 | vstinner | set | messages: + msg366171 |
| 2020-04-10 00:16:19 | Eric Cousineau | set | messages: + msg366093 |
| 2020-04-09 21:06:35 | pablogsal | set | messages: + msg366088 |
| 2020-04-09 20:27:13 | Eric Cousineau | set | messages: + msg366087 |
| 2020-04-09 20:26:21 | Eric Cousineau | set | messages: + msg366086 |
| 2020-04-09 16:25:50 | pablogsal | set | messages: + msg366078 |
| 2020-04-09 16:23:46 | pablogsal | set | messages: + msg366077 |
| 2020-04-09 16:01:40 | Eric Cousineau | set | messages: + msg366076 |
| 2020-04-09 15:27:51 | vstinner | set | messages: + msg366069 |
| 2020-04-09 15:20:21 | vstinner | set | nosy:
+ pitrou, vstinner, pablogsal messages: + msg366066 |
| 2020-04-09 15:00:32 | Eric Cousineau | create | |
