Issue1057

old style classes create a PyFinalizableInstance when a__del__ method is 
defined (unfortunately there's even problems with that to: http://bugs.jython.org/issue1634167 )

new style classes don't do anything similar however, thus __del__ 
methods are never called

this prevents test_descr's delhoook and subtype_resurrection from 
passing

Ideally we'd also warn the user if they apply a __del__ after the fact 
(see #1634167)

But subtype resurrection is evil anyway ;)

I think this is a major and longstanding departure from CPython's behavior. Can it be fixed for Jython 2.7 please?

Blocker for 2.7, we really should fix

Perhaps we can emulate this support via tracking with weak references, then dequeueing the corresponding reference queue and calling __del__, vs using a finalizer method that calls __del__. Such an approach would also support possible resurrection.

The tricky bit will be to figure out how to making object construction even more expensive than it already is.

As was discussed in #1634167, dynamically adding __del__ to the class of with some existing object x would not call x.__del__(), since we do not track the objects of a given class. A RuntimeWarning should be generated in this case like PyPy (http://bugs.jython.org/msg5245).

In dialogue with Jim, I developed a solution based on what was referred to as FinalizeGuardian in http://bugs.jython.org/issue1634167.
Appended is only a draft and not yet an actual fix, because I wanted to discuss some of its implications here first. In addition to the appended stuff one would have to change all the XxxDerived classes as follows:

Let XxxDerived-classes implement FinalizablePyObject
and add a member
FinalizeTrigger finalizeTrigger;
like I demonstrated with PyFinalizableInstance and PyFinalizableObject.
 
In every constructor must be appended:
PyType self_type=getType();
PyObject impl=self_type.lookup("__del__");
if (impl!=null) finalizeTrigger = FinalizeTrigger.makeTrigger(this);
 
And finally add a __del__-method:
public void __del__() {
  PyObject impl = self_type.lookup("__del__");
  if (impl != null) {
    impl.__get__(this,self_type).__call__();
    FinalizeTrigger.resurrectOnDemand(this);
  }
}
 
IMHO, this should fix it; at least with the same constraint as in the fix of issue 1634167. The nice thing is, that for instances of XxxDerived that don't have a __del__ finalizer, Java won't have to create an expensive Java-finalizer, since finalizeTrigger just stays null.

I developed this approach also with JyNI-integration as a design goal. Especially the methods FinalizeTrigger.prohibitFinalize and FinalizeTrigger.allowFinalize might seem inherently evil as they allow to make certain objects temporarily immortal. To implement a clean garbage collection in JyNI, it sometimes needs to resurrect a gc'ed object in a controlled way and in this case, the finalizer, if any, should not be called by a FinalizeTrigger, nor should the object be reclaimed. JyNI needs this time to check, whether native resources exist that still need the corresponding object.
I reasoned carefully about this and do not see a feasible alternative to this approach. However, I will do my best to let such situations arise as rarely as possible.
Again, if JyNI is not used, this evil stuff won't be done anyway. The implementation just ensures that it can be done without breaking Jython's own finalization process.

As an implication of this, it must be forbidden for subclasses of PyObject to implement finalize(), which I enforce by adding

protected final void finalize() throws Throwable {}

to it. An appropriate adjustment for PyFinalizableInstance is included.
However, it might break existing third-party implemented subclasses of PyObject, if they implement finalize. I still recommend this change for Jython 2.7., as such third-party implementations would be trivial to fix. Additionally, I suppose that they are rather rare, if existent at all, since there is a common consens to avoid using finalizers.

Another note: The attached implementation would allow to reproduce CPython's behavior regarding to repeated object resurrection. Prior to 3.4 it would also repeatedly call the finalizer, which is not emulated by current solution for http://bugs.jython.org/issue1634167. My proposed solution would fix this for http://bugs.jython.org/issue1634167 as well.
However, Jim pointed out that it might be better to directly stick to Cpython's >= 3.4 behavior (i.e. call __del__ only once, like Java would do and thus prohibit repeated resurrection). The proposed solution would support both variants and switching between them is a trivial adjustment. One could even make it configurable.

I did not yet test this approach with Jython, but I tested the used principles with some dummy classes in detail. Depending on your reply I will do actual test with Jython and work it out as an actual patch.

I unfortunately discovered that the technique to automatically detect resurrection (used in resurrectOnDemand) only works, if there occurs a gc-run before the resurrected object becomes unreachable again.
I overlooked this behaviour first (sorry for that!), because in my tests I always called System.gc multiple times to assure it ran at least once.
Since in usual cases one does not control when gc runs, it would actually be undefined whether repeated finalization would work or not, wich I consider worse than having it never work. One would have to detect a gc-run and re-run every time, which is unreasonably expensive in best case and maybe even impossible. So I would stick to the suggestion to directly perform Python 3.4 behavior here.

 
Next topic.

I forked Jython to implement and test this. Besides other tests, I explored how CPython behaves if an instance (not the class!) acquires a finalizer.
 
A = SomeClass()
A.__del__ = blah
 
There are issues with binding the method to the instance, but basically a __del__ method is not necessarily required to be bound to its instance.
However it appears that instance-acquired finalizers are supported for old style classes in CPython, but not for new style.

We could emulate this behavior in Jython for old style classes. The technique with FinalizeGuardian/FinalizeTrigger just requires that the Java object holds the single reference to a FinalizeTrigger, which in turn contains the actual finalizer. To support instace-acquired __del__, the PyInstanceClass would need to be enhanched by such a memeber, which would simply be null for all instances without __del__.
If we chose this variant, there would be no more need for the PyFinalizableInstance class any more and it could be removed or deprecated.

But maybe we would not want to increase the memory demand for old style python objects by one reference per object in general.

To avoid this, one could add the FinalizeTrigger to the instance's dictionary instead. This would only increase memory usage for objects that need it, but would somehow mess up their dictionary with an object that is irrelevant from Python view. Additionally one would have to implement FinalizeTrigger as a PyObject, which also increases memory demand. And there would be checks necessary to move the trigger to the new dict if the dict is replaced.

So please let's decide how to proceed. Shall instance-acquired finalizers be supported or not, considering the given costs?
If so, should it be done via a member or via the dictionary?
Or maybe someone has a better idea how to hide a reference to a FinalizeTrigger in the PyInstance I did not consider yet?

I finally decided to have finalizeTrigger in every PyInstance and removed PyFinalizableInstance. Benefits:

- It is possible for instances to acquire __del__ finalizers like in CPython

- If a class acquires a __del__ method this will be only active for instances created after the calss acquired the finalizer. Now it is possible to at least manually tell already existing instances to be finalizable:
Add to import section:
if platform.system() == "Java":
	from org.python.core.finalization import FinalizeTrigger

After a class acquired a finalizer, one can call
if platform.system() == "Java":
	FinalizeTrigger.ensureFinalizer(alreadyExistingInstance)

- CPython prior to 3.4 allows repeated object resurrection, i.e. calls finalizers again, if an object was already gc'ed, resurrected and gc'ed again. This behavior cannot be reproduced by Java automatically. So the fix behaves like CPython >= 3.4 by default. However, one can use
if platform.system() == "Java":
	FinalizeTrigger.ensureFinalizer(resurrectedInstance)
to tell Java manually that a repeated finalization is intended. This way one can fix code that depends on CPython < 3.4 behavior.


I propose to forbid PyObjects o have a finalize method.  This is done by adding a final empty implementation. This has no performance impact, since empty finalizers are optimized away. See http://www.javaspecialists.eu/archive/Issue170.html

This forces a clean use of the new finalization API. However, without this constraint everything should still work, except that one cannot fix repeated finalization manually. The real problems only arise if JyNI is used. It will cause that Java-finalizers might be called too early, i.e. when objects are still needed. This is a side-effect of its way to deal with garbage collection for extensions.

Given that an easy fix for PyObjects that need Java finalizers is available (it is explained by detailed instructions in FinalizablePyObject's javadoc) and that Jython 2.7 is a significant step, I recommend to undertake this break here.

If you should not agree, I would suggest to make it fire a compiler-warning (can anyone tell me how to do this?) stating that problems with JyNI will probably arise, if Java-finalizers are used. In that case I would recommend to forbid finalizers at least for Jython 3+ versions.


Feel free to review the patch at https://bitbucket.org/stewori/jynithon/commits/288366399aeda6832b36cc5be432a2b878d09139?at=default

It passes regrtests with no notable difference to Jython without the fix (which actually fails 25-27 regrtests on my system).

I added a new test demonstrating the new capabilities as Lib/test/test_finalizers.py

According to the patch guidelines https://wiki.python.org/jython/PatchGuidelines, I should create an svn diff file and provide it here. These guidelines appear outdated to me, which is why I created a mercurial commit in bitbucket instead. Please let me know, if the svn diff is still needed or where I can find an updated patch guide.

Fixed as of http://hg.python.org/jython/rev/6953968b2638