RFC 7038914: VM could throw uncaught OOME in ReferenceHandler thread

[David Holmes]

Hi Thomas,

On 9/05/2013 1:28 AM, Thomas Schatzl wrote:
> Hi,
>
>    please review the latest webrev for this patch that is available at
>
> http://cr.openjdk.java.net/~tschatzl/7038914/webrev.2/
>
> As mentioned, it incorporates the fix and reproducer from Peter Levart.

Fix is fine.

I'm not sure about the test (sorry I didn't pay it attention earlier). 
Have you instrumented the code to verify that the test actually triggers 
an OOME? It may be possible that if the OOME did kill the reference 
thread that we wouldn't necessarily detect it using the sleeps etc. Also 
I don't understand the need for the actual weakRef usage and System.gc() 
etc. If the heap is full then the interrupt will wake the reference 
handler thread and the allocation will trigger the OOME. It doesn't 
matter if there are any references to process. The main logic might then 
reduce to:

  referenceHandlerThread.interrupt();
  for (int i = 0; i < 10; i++) {
    if (!referenceHandlerThread.isAlive())
        throw new Exception("Reference-handler thread died");
    Thread.sleep(1000);
  }

which just gives it 10s to die else assumes all ok. ?

But I can live with existing test (it just might vacuously pass)

Cheers,
David

> Bugs.sun.com:
> http://bugs.sun.com/view_bug.do?bug_id=7038914
>
> JIRA:
> https://jbs.oracle.com/bugs/browse/JDK-7038914
>
> Testing:
> JPRT, with the new reproducer passing
>
> In need of reviewers and a sponsor for this patch; as mentioned earlier
> I will set Peter (plevart) as author for this patch, i.e. send a
> patchset to the sponsor for this change with that author set.
>
> Thanks,
> Thomas
>