Allow objects to decide if they can be collected by GC

[kristjanvalur]

BPO	9141
Nosy	@tim-one, @loewis, @rhettinger, @amauryfa, @pitrou, @kristjanvalur, @asvetlov, @JimJJewett, @serhiy-storchaka, @phmc
Files	gc_collectable.patch ob_is_gc.patch ob_is_gc.patch

^{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 2018-06-04.10:11:46.014>
created_at = <Date 2010-07-02.11:30:10.750>
labels = ['interpreter-core', 'type-feature']
title = 'Allow objects to decide if they can be collected by GC'
updated_at = <Date 2018-06-04.10:11:46.013>
user = 'https://github.com/kristjanvalur'

bugs.python.org fields:

activity = <Date 2018-06-04.10:11:46.013>
actor = 'kristjan.jonsson'
assignee = 'none'
closed = True
closed_date = <Date 2018-06-04.10:11:46.014>
closer = 'kristjan.jonsson'
components = ['Interpreter Core']
creation = <Date 2010-07-02.11:30:10.750>
creator = 'kristjan.jonsson'
dependencies = []
files = ['17838', '25131', '25142']
hgrepos = []
issue_num = 9141
keywords = ['patch', 'needs review']
message_count = 44.0
messages = ['109099', '109100', '109112', '109113', '109226', '109229', '109240', '109241', '109247', '109248', '109265', '109802', '109803', '109804', '109807', '109810', '110973', '110975', '157464', '157492', '157571', '157621', '157657', '157658', '157681', '157682', '157685', '157688', '157689', '157714', '157719', '157722', '157723', '158510', '158537', '158538', '158540', '158542', '158543', '158560', '158561', '318537', '318582', '318641']
nosy_count = 13.0
nosy_names = ['tim.peters', 'loewis', 'rhettinger', 'amaury.forgeotdarc', 'pitrou', 'kristjan.jonsson', 'dstanek', 'stutzbach', 'asvetlov', 'Jim.Jewett', 'serhiy.storchaka', 'pconnell', 'isoschiz']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue9141'
versions = ['Python 3.3']

[kristjanvalur]

The GC module currently relies only on the presence of a __del__ method to decide if an object can be safely collected at all. Also, there is a special case for generator objects.

This patch allows any object to call an api during its traversal,
PyObject_GC_Collectable(), to indicate whether it is fit to be collected at this time, overriding any presence of a __del__ method or not.

This mechanism is being put in place in stackless python 2.7 because tasklets cannot always be collected depending on their runtime state, and I thought this might be a useful addition for regular python, especially since there already is such a dynamic special case for generator objects.

[pitrou]

First, what's the use case? Just backporting an internal Stackless API isn't a reasonable request.
Second, this tells that there *is* a finalization function, but not what the function is. How is the GC supposed to find it?
Third, the implementation looks quite suboptimal. Better define a new slot method, such as tp_needs_finalizing, or tp_get_finalizer.

[kristjanvalur]

1. The use case is any C extension that may need to run non-trivial code when being deleted, but where this can not be statically known. An example in the code today is generators, and the PyGen_NeedsFinalizing() function, doing exactly this. This is an attempt to make this functionality available to any C extension.

2. No, this gives a C extension the oppertunity to say: It's okay to delete me from GC, or it's not okay to delete me from GC. There is no need to specify any particular finalizing function, it will be invoked when the refcount goes to 0, just as it happens with __del__(), or as it happens with PyGen objects.

3. This code is only invoked for garbage deemed collectable. As such it is not on any critical path, most gc collections don't actually find any garbage. The cost of a few extra indirect function calls is likely to drown in the memory allocator overhead when the objects are released.

Also, this is designed to be a minimum impact patch. We really shouldn't be creating new slot methods when it can be avoided.

[pitrou]

1. The use case is any C extension that may need to run non-trivial
   code when being deleted, but where this can not be statically known.

tp_del is IMO a bad place to do it. I'd recommend tp_dealloc instead,
precisely so that you don't end up with uncollectable objects tied to
internal OS structures or other non-trivial resources.

(also, tp_del seems to have problems with subclassing. I don't remember
the specifics)

3. This code is only invoked for garbage deemed collectable. As such
   it is not on any critical path, most gc collections don't actually
   find any garbage. The cost of a few extra indirect function calls is
   likely to drown in the memory allocator overhead when the objects are
   released.

Ok.

[kristjanvalur]

I don't understand the tp_del / tp_dealloc argument here. This is really not the concern of gc. Gc only goes and finds unreachable objects, and if they are deemend collectable, makes them go away by clearing their references.
GC only cares that during the final release of any objects in a reference cycle, no non-trivial code may run (for various technical reasons). So, objects deemed "sensitive" in this way, are left alone (and put in gc.garbage instead).
GC has no other knowledge of how objects behave, if or when any finalizers are called. The only thing gc does is calling Py_DECREF (through the tp_clear slot). Everything else is up to the objects.

The intent of this patch is for objects themselves to be able to provide better information to GC as to whether they are too "sensitive" to be cleared by GC, rather than GC relying solely on the presence of a __del__ method, or using the builtin special case knowledbe for PyGen objects.

Whether finalizers are called from tp_del or tp_dealloc is beyond the scope of the gc module or, indeed, this patch.

[pitrou]

GC only cares that during the final release of any objects in a
reference cycle, no non-trivial code may run (for various technical
reasons). So, objects deemed "sensitive" in this way, are left alone
(and put in gc.garbage instead).

Which is really the problem, and why you should prefer tp_dealloc over
tp_del.
(unless you want gc.garbage to fill up too easily)

[kristjanvalur]

I'm confused, Antoine. _I_ am not preferring anything. I am not modifying the way finalizers are called. I'm merely providing a way for objects to signal that they _have_ (or don't have) non-trivial finalizers.

A finalizer _may_ be tp_del, but it may be any code that is called as part of tp_dealloc(). This patch allows an object to say: Calling tp_dealloc() is currently unsafe from a gc.collect() run.

[pitrou]

A finalizer _may_ be tp_del, but it may be any code that is called as
part of tp_dealloc(). This patch allows an object to say: Calling
tp_dealloc() is currently unsafe from a gc.collect() run.

Well, I don't know what you mean. tp_dealloc should always be "safe" and
is never checked for by the GC.

By the way, tp_del isn't even documented, which reinforces my point that
it's not really for use by third-party extension types.

I think we are reaching the point where, if you want this to happen, you
should ask for opinions on python-dev, because I really don't understand
which problem you are trying to solve.

[kristjanvalur]

tp_del() is generally invoked by tp_dealloc(). See for example typeobject.c:849. gc.collect() never invokes tp_del() directy, that is left for the objects themselves (as part of tp_dealloc())

Now, gc already knows that if tp_del is present, it cannot cause break the cycle, (for various technical reasons, one of them being that tp_del would be invoked with various dependency objects already nerfed by tp_clear()).

But this is not always enough:

1. it may be too pessimistic. Sometimes the tp_del method doesn't do anything significant, or isn't even called, depending on runtime state, so it is safe for gc to clear the object (see for example genobject.c:30).
2. Sometimes finalization behaviour is not defined by a tp_del method at all, but rather some custom code in tp_dealloc.

All this patch does, is to generalize the mechanism already provided for genobject.c (by PyGen_NeedsFinalizing()), to any object: Any object can signal to gc that: a) it is ok to collect a cycle with me in it, or b) no, it is unsafe to do so. With this patch in place, PyGen_NeedsFinalizing() no longer needs to be a special case in gcmodule.c.

Yes, I'll send a message to python-dev.

[pitrou]

All this patch does, is to generalize the mechanism already provided
for genobject.c (by PyGen_NeedsFinalizing()), to any object: Any
object can signal to gc that: a) it is ok to collect a cycle with me
in it, or b) no, it is unsafe to do so. With this patch in place,
PyGen_NeedsFinalizing() no longer needs to be a special case in
gcmodule.c.

Adding an API function and an additional traversal pass to the GC just
for the sake of removing a special case doesn't seem reasonable to me.
That's why I'm asking what specific problem you are trying to solve.
(rather than simply trying to make things "nicer")

[stutzbach]

Kristján,

Can you compare and contrast your approach with calling PyObject_GC_UnTrack and PyObject_GC_Track to mark the object as uncollectable/collectable?

[kristjanvalur]

Hi Antoine.
I am not adding another traversal pass. I am modifying the function that gets called for every object about to be collected, to call the object's traversal function. This is hardly an extra "pass" and is done only for objects that are about to be collected (i.e. only in the rare case).

The particular case is this: I have objects (tasklets) that depending on their runtime state, need to run a finalizer or not. If not, they can be collected. If they do, then they end up on gc.garbage.

This is exactly the same case as with generators. But instead of adding yet another special case into stackless python, I realized that it may be up to objects' runtime state whether they need to run a finalizer or not. So, I wrote a more generic way and decided to contribute it to C Python, so that a future developer, having a C extension that only sometimes needs to run a finalizer, could have a way to cleanly deal with garbage collection. This is twice now that this problem has been seen in python. The first time, a special case was coded into gcmodule. This second time, I'm offering a more generic solution.

Now, I have a number of useful improvements and additions to C python sitting in our repository, the result of over 7 years work with Python at CCP games. I have another one sitting and waiting that adds useful functionality to gcmodule. Useful in my opinion anyway, and worth at least being kept for posterity as a patch in the tracker. But since these patches of mine seem to be met with rather limited enthusiasm, I wonder if I should be bothering.

[kristjanvalur]

Hi Daniel.
Your message was classified as spam, I have no idea why, but this is why I only noticed it now.

Your suggestion is interesting, I hadn't thought of that. Yes, it is possible to use the track/untrack functions (I think), but that would mean that you would have to monitor your object for every state change and reliably detect the transition from one state to another. Being able to query the current state and let gc know: "No, I cannot be collected as I am now" is a much more robust solution from the programmers perspective.

A further difference is this: If an object isn't tracked, it won't be collected if it is part of a cycle, but it will not be put in gc.garbage either. In effect, it will just remain in memory, unreachable, with no chance of it ever being released.

In contrast, a generator (or in my case, a tasklet) that needs to have a finalizer run when it goes away, will end up in gc.garbage, which a dilligent application will visit once in a while in order to clean it out. I'm not sure how you would clear out a generator like that, but in Stackless python, you could do something like this once in a while:
for g in gc.garbage:
if isinstance(g, stackless.tasklet):
g.kill()
del gc.garbage[:]