On Mon, 7 Dec 2020 10:44:56 GMT, Per Liden <[email protected]> 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

Reply via email to