RFR: 8255987: JDI tests fail with com.sun.jdi.ObjectCollectedException [v2]

[Chris Plummer]

On Mon, 7 Dec 2020 10:44:56 GMT, Per Liden <pliden at openjdk.org> wrote:

>> test/hotspot/jtreg/vmTestbase/nsk/jdi/ReferenceType/instances/instances002/instances002.java line 194:
>> 
>>> 192:                     debuggee.resume();
>>> 193:                     checkDebugeeAnswer_instances(className, baseInstances);
>>> 194:                     debuggee.suspend();
>> 
>> Before the changes in this PR, what was triggering the (expected) collection of the objects?
>
> @plummercj Nothing was explicitly triggering collection of these objects. However, the test is explicitly checking the number of objects "reachable for the purposes of garbage collection" in `checkDebugeeAnswer_instances()`. The tests sets up a breakpoint (with SUSPEND_ALL), which suspends the VM. Then it creates a number of new instances and expects these to be weakly reachable. However, with this change, suspending the VM will make all objects "reachable for the purposes of garbage collection". So, to let the test continue to create objects which are weakly reachable we need to first resume the VM, create the new instances, and then suspend it again.
> 
> @dholmes-ora I have no idea why these tests are so different. The VM suspend is implicit in the breakpoint in this test, which is set up using SUSPEND_ALL.

Ok, I understand now. `ReferenceType.instances()` only counts objects "reachable for the purposes of garbage collection". This change in behavior does concern me a little bit. I think the expectation is that the instances created by `ClassType.newInstance()` will not show up in this count unless `disableCollection()` is called, even when under a "suspend all". Clearly that's the expectation of this test, so the question is whether or not it is a reasonable expectation.

Note that `ClassType.newInstance()` says nothing about the state of the returned object w.r.t. GC. It makes no mention of the need to call `disableCollection()` before resuming the VM, so I guess this gives us some wiggle room. However, the argument against the object being strongly reachable is comes from user asking the question "who has the strong reference that makes it strongly reachable?". It's not obvious to the user why there is a strong reference, and why it seemingly goes a way once `VM.resumeAll()` is called.

I still think overall this is the right approach (baring a better approach being presented), but we may need to include some spec clarifications, and be prepared for some push back if this breaks anything.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1595