RFR 8203174: [Graal] JDI tests fail with Unexpected exception: com.sun.jdi.ObjectCollectedException

[Chris Plummer]

Hi Daniil,

Thank you for stabilizing these tests.

I don't think your comment changes for the following are correct:

  120             // instances created in this way aren't reachable for 
the purposes of garbage collection,
  121             // to make it reachable call enableCollection() for 
this objects
  122             ArrayReference arrayReference = 
debuggee.createArray(arrayType, arraySize);

They actually are reachable now that you are calling createArray(). 
Maybe you can just get rid of the comment now since its purpose was to 
explain why disableCollection() was being called, and this is no longer 
being done (at least no inline here).

In referringObjects001.java you removed the following comment:

  155         // Note! This test is broken, in the sense that it 
incorrectly assumes
  156         // that no GC can happen before it walks the heap. In 
practice, it seems
  157         // to only affect this test when using ZGC. However, this 
test will also
  158         // fail when using other GCs if an explicit GC is done here.

I guess the ZGC team must have run into failures with this test. I'm not 
so sure your changes fix this for ZGC. Don't the ZGC threads continue to 
run even after you call debugee.suspendOtherThreads()? Also, I don't see 
how the suspendOtherThreads helps here. createInstance() will always 
resume all threads before calling the constructor and then suspend all 
threads afterwards. So suspending beforehand doesn't seem like it would 
help at all.

  657     public  ArrayReference createArray(ArrayType array, int 
arrayLength) {
  658         vm.suspend();
  659         try {
  660             ArrayReference arrayRef = array.newInstance(arrayLength);
  661             arrayRef.disableCollection();
  662             return arrayRef;
  663         } finally {
  664             vm.resume();
  665         }
  666
  667     }

Line 666 can be removed.

thanks,

Chris

On 6/22/18 7:13 PM, Daniil Titov wrote:
> Please review the changes that fix 15 tests failures when running with Graal.
>
> The main problem here is that the tests assume that no garbage collection happens before the tests complete their checks. The other problem is that the tests could not strictly distinguish the objects created by the debuggee on the request from the test from the objects of the same class created by the debuggee VM on its own.  In case when java.lang.String or arrays of primitives are used in the tests the number of objects the test operates could significantly exceeds the number of the objects the test actually created and iterating over all of them results in the timeout.
>
> The fix ensures that the object references created in the tests are protected from garbage collection or modifies the test to expect an ObjectCollectedException where necessary. In some cases, the fix ensures that only the main thread of the debuggee is running to prevent the garbage collector thread to create objects of the same class the test is working with and avoid timeouts.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8203174/webrev.01
> Bug: https://bugs.openjdk.java.net/browse/JDK-8203174
>
> Thanks,
> Daniil
>
>