Still looking for a reviewer for this please.
Thanks,
David
On 5/07/2013 9:22 AM, David Holmes wrote:
Hi Peter,
On 2/07/2013 5:19 PM, Peter Levart wrote:
Looking at original code once again, I think this was actually a bug.
The WeakReference instance constructed in (old) line 82, can be GCed
right away, since nobody is using the local variable after assignment.
Of course. Doh! I was to busy thinking about the lifetime of the
referent object to consider the reference itself.
If WeakReference is GCed it can not be enqueued. The promotion of local
variable into a field is one way to fix this. The other would be to use
the local variable somewhere down the code path, like for example in a
final throw statement:
110 throw new IllegalStateException("Reference Handler thread
stuck. weakRef.get(): " + weakRef.get());
This would also reveal some more info about the WeakReference when
there's no sure answer after 10 seconds and could be added to the test
anyway.
Okay I've modified the test as suggested. Updated webrev at same location:
http://cr.openjdk.java.net/~dholmes/8016341/webrev/
In testing it though I simply exposed the remaining flaws in the
ReferenceHandler code. We can still kill the ReferenceHandler thread
with an OOME when it tries to load the Cleaner class (running with a 5M
heap triggers this nicely if you use G1):
// Fast path for cleaners
if (r instanceof Cleaner) {
((Cleaner)r).clean();
continue;
}
and if that passes the clean() might throw OOME (it internally tries to
do a System.exit if an exception occurs but will likely encounter
another OOME trying to create the PrivilegedAction).
Even the:
ReferenceQueue q = r.queue;
if (q != ReferenceQueue.NULL) q.enqueue(r);
might throw OOME because enqueue() might have to load the FinalReference
class.
So really catching the OOME around the wait() only patches a small hole.
We can't simply put a try/catch in the for(;;) loop because that doesn't
address the problem that if the class loading throws OOME then
subsequent attempts to load that class will also fail. We would have to
preload all possible classes. Even then we might just send the
ReferenceHandler thread into a busy loop of throwing OOME catching it
and retrying!
So I can fix the test to deal with the Xcomp issue but we may still see
intermittent failures, and the ReferenceHandler thread may still die
from OOME.
David
-----
Regards, Peter
On 07/02/2013 06:38 AM, David Holmes wrote:
This recently added test was found to fail under some conditions -
namely client compiler with -Xcomp. It seems that the use of all local
variables enabled the compiler to optimize things in a way that
stopped the weakref from being enqueued as expected. Simple fix was to
make the weakref a field.
http://cr.openjdk.java.net/~dholmes/8016341/webrev/
Thanks,
David
