Hi Peter,
I've always assumed that the arguments could not be gc'd at least until
the method returns
in part because the caller was holding references. But I suppose in a
completely inlined
case an optimizer might try to make more relaxed conclusions.
Since the new object has not yet been published, the reachability of
objects in a constructor
must have some additional rules to keep objects alive until it is published.
On 10/21/2015 8:13 AM, Peter Levart wrote:
Not so fast, there's more...
On 10/21/2015 01:50 PM, Peter Levart wrote:
Hi Roger,
I think I have another race ;-)
This time it is more theoretical than practical. If we take a look at
say PhantomCleanable constructor:
public PhantomCleanable(T referent, Cleaner cleaner) {
super(Objects.requireNonNull(referent),
getCleanerImpl(cleaner).queue);
this.cleanerImpl = getCleanerImpl(cleaner);
insert();
}
...somewhere in the getCleanerImpl(cleaner) method where 'cleaner' is
last dereferenced and before assignment of extracted CleanerImpl to
this.cleanerImpl, 'cleaner' is not needed any more. If it is not
referenced from anywhere else in the program and JVM can prove it
will not be used any more, JVM is free to declare it phantom
reachable. The PhantomCleanable that is used to track the 'cleaner'
can therefore be enqueued and the cleaner thread can process it,
effectively emptying the 'phantomCleanableList' before this
PhantomCleanable constructor insert()s itself into the
'phantomCleanableList'. The cleaner thread can therefore exit the
loop and terminate prematurely.
The fix would be to reverse the last two statements of the
XxxCleanable constructor(s):
public PhantomCleanable(T referent, Cleaner cleaner) {
super(Objects.requireNonNull(referent),
getCleanerImpl(cleaner).queue);
insert();
this.cleanerImpl = getCleanerImpl(cleaner);
Except that insert() uses cleanerImpl to know what list to insert it in.
(It could be passed as an argument; if the concern about the Cleaner
becoming phantomReachable at that moment was valid)
}
... or to add a call to the (not yet in jdk9 but coming soon)
reachabilityFence():
public PhantomCleanable(T referent, Cleaner cleaner) {
super(Objects.requireNonNull(referent),
getCleanerImpl(cleaner).queue);
this.cleanerImpl = getCleanerImpl(cleaner);
insert();
Reference.reachabilityFence(cleaner);
}
...similar race could be devised around the reachability of
'referent'. As soon as it is assigned to the Reference.referent field,
it is not needed any more, so it can be declared
(phantom|weakly|softly)-reachable by JVM before the XxxCleanable
instance is insert()ed causing it to be attempted to be remove()d
before it is insert()ed. The full fix therefore needs this:
public PhantomCleanable(T referent, Cleaner cleaner) {
super(Objects.requireNonNull(referent),
getCleanerImpl(cleaner).queue);
this.cleanerImpl = getCleanerImpl(cleaner);
insert();
Reference.reachabilityFence(cleaner);
Reference.reachabilityFence(referent);
}
Someone more familiar with the constructor/gc interactions might need to
weigh in on this.
There must be some accommodation or lots of existing code in
constructors would be/have broken.
Thanks, Roger
Regards, Peter
Regards, Peter
On 10/20/2015 08:28 PM, Roger Riggs wrote:
Sorry for the silence, JavaOne preparations and the availability of
folks who wanted
to review have stretched things out.
The Cleaner API was very simple and saw feature creep as the ideas
for how it might be used
were explored. There are concerns about committing to supporting
subclassable
CleanableReferences in all future JDK versions before there had been
a chance to
see how if they would be useful and necessary to address the need to
reduce the
use of finalization within the OpenJDK and beyond.
Recent updates:
- The Cleaner implementation classes and the CleanableReference
abstract classes are
now in the jdk.internal.misc package and are available within the
java.base module.
- The Cleanable.clear method has been dropped; there is no current
use case.
Since the CleanableReferences extend Reference, clear() is
available when subclassing.
- The tests have been extended to cover the exported and internal APIs
The Runnable thunk version is very convenient to code but does
transparently create
an additional object to hold the bindings.
As the Cleaner is applied to the various uses of finalize we'll see
how they would be used
and can re-evaluate the exported API needs.
Updated Javadoc:
http://cr.openjdk.java.net/~rriggs/cleaner-doc/
Updated Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
Thanks, Roger
