Repository navigation
Expose public spelling of _PyGC_FINALIZED and _PyGC_SET_FINALIZED? #84421
Description
| BPO | 40240 |
|---|---|
| Nosy | @pitrou, @vstinner, @pablogsal, @EricCousineau-TRI, @iritkatriel |
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:
assignee = None
closed_at = <Date 2021-06-15.12:51:26.164>
created_at = <Date 2020-04-09.15:00:32.544>
labels = ['expert-C-API', '3.8', 'type-feature', '3.7', '3.9']
title = 'Expose public spelling of _PyGC_FINALIZED and _PyGC_SET_FINALIZED?'
updated_at = <Date 2021-06-15.12:51:26.163>
user = 'https://github.com/EricCousineau-TRI'bugs.python.org fields:
activity = <Date 2021-06-15.12:51:26.163>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-15.12:51:26.164>
closer = 'vstinner'
components = ['C API']
creation = <Date 2020-04-09.15:00:32.544>
creator = 'Eric Cousineau'
dependencies = []
files = []
hgrepos = []
issue_num = 40240
keywords = []
message_count = 13.0
messages = ['366059', '366066', '366069', '366076', '366077', '366078', '366086', '366087', '366088', '366093', '366171', '395870', '395877']
nosy_count = 5.0
nosy_names = ['pitrou', 'vstinner', 'pablogsal', 'Eric Cousineau', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40240'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9']
Activity
EricCousineau-TRI commentedon Apr 9, 2020
Motivated by this downstream project issue that I am working on:
RobotLocomotion/drake#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?
vstinner commentedon Apr 9, 2020
See also bpo-40241: "[C API] Make PyGC_Head structure opaque, or even more it to the internal C API".
vstinner commentedon Apr 9, 2020
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:
_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 8766cb7.
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.
EricCousineau-TRI commentedon Apr 9, 2020
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.
pablogsal commentedon Apr 9, 2020
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.
pablogsal commentedon Apr 9, 2020
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.
EricCousineau-TRI commentedon Apr 9, 2020
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!
EricCousineau-TRI commentedon Apr 9, 2020
Er, to clarify: "PEP-442 had that effect" => "PEP-442 had the effect of making it resurrectable only once, but not more than that."
pablogsal commentedon Apr 9, 2020
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.
EricCousineau-TRI commentedon Apr 10, 2020
Aye. Using a workaround for now ("leak" the object by incrementing the refcount on first resurrection):
RobotLocomotion/pybind11#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!
vstinner commentedon Apr 11, 2020
New changeset f13072b by Pablo Galindo in branch 'master':
bpo-40241: Add PyObject_GC_IsTracked and PyObject_GC_IsFinalized to the public C-API (GH-19461)
f13072b
iritkatriel commentedon Jun 15, 2021
Can this be closed now? It seems that the query API was added and the set API was rejected.
vstinner commentedon Jun 15, 2021
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.