Testing no memory leak occurs via references

[Aleksei Ivanov]

Thank you, Stuart and Roger, for the suggestion.

I didn't know about ForceGC class. We should use the common library, I 
have updated my PR [7] with ForceGC.


Albert and Thomas suggested using WhiteBox API. It would probably work 
too, however, I decided ForceGC is simpler and easier to use, so I went 
for it. It does its job and it's used in core-libs tests.


Thanks to everyone who provided their input, I appreciate your help.

-- 
Regards,
Alexey

[7]  https://github.com/openjdk/jdk/pull/12594

On 03/03/2023 22:54, Stuart Marks wrote:
>
> As Roger mentioned, there is a ForceGC utility in the test library:
>
>     test/lib/jdk/test/lib/util/ForceGC.java
>
> and it's used in a variety of places in the core libs tests. 
> Essentially it sets up a PhantomReference and a ReferenceQueue and 
> runs System.gc() in a loop. I'd strongly recommend using this in 
> preference to allocating a lot of memory in order to provoke 
> OutOfMemoryError. That technique has historically been a cause of test 
> flakiness, and it still is, as you've discovered.
>
> There is also MemoryMXBean.gc(), which does the same thing System.gc() 
> does -- it calls Runtime.getRuntime().gc().
>
> It's true that System.gc() may sometimes be ignored -- for instance if 
> Epsilon GC is enabled -- but for practical purposes, on Hotspot using 
> a standard collector, calling it will eventually cause garbage 
> collection and reference processing.
>
> If at some point the behavior provided by System.gc() is inadequate 
> for our testing, we'll need to plumb a JDK-specific interface that has 
> stronger semantics, and then convert ForceGC to use it so that 
> individual tests won't have to be updated.
>
> There are still some tests that allocate lots of memory in order to 
> provoke OOME and consequently reference processing. They probably need 
> to be run in /othervm mode in order to set custom heap sizes and to 
> avoid interfering with other tests. It would be interesting to see if 
> those could be adjusted to use something ForceGC so that they can 
> share the JVM with other tests and also avoid allocating lots of memory.
>
> s'marks
>
>
> On 3/3/23 10:02 AM, Aleksei Ivanov wrote:
>> Hello,
>>
>> In clientlibs, there's occasionally a need to verify an object isn't 
>> leaked. For this purpose, WeakReference or PhantomReference is used.
>>
>> Then, we need to make the reference object be cleared, so a GC cycle 
>> need to be triggered. The common approach is generating 
>> OutOfMemoryError, catching it and verifying whether the reference is 
>> cleared.
>>
>> Some tests use a utility method regtesthelpers/Util.generateOOME [1].
>>
>> For example, these tests follow the above approach:
>> https://github.com/openjdk/jdk/blob/master/test/jdk/javax/swing/border/TestTitledBorderLeak.java
>> https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/List/ListGarbageCollectionTest/AwtListGarbageCollectionTest.java
>>
>>
>> The AwtListGarbageCollectionTest.java test started to fail pretty 
>> often in the end of January 2023.
>>
>> I followed a piece of advice provided in a JBS comment for 
>> JDK-8300727 [2] and replaced generating OOME with a simple call to 
>> System.gc() along with adding a loop for re-trying.
>>
>> The specification for System.gc() [3] mentions that this call can be 
>> ignored, which started a discussion in the PR #12594 [4] that 
>> System.gc() should not be used, at the very least without generating 
>> OOME in addition to invoking System.gc().
>>
>> At the same time, many tests for Reference objects, such as 
>> ReferenceEnqueue.java [5] and PhantomReferentClearing.java [6], rely 
>> solely on System.gc.
>>
>>
>> What would be your recommendation? Are there best practices in 
>> core-libs and hotspot for testing for memory leaks that clientlibs 
>> should follow?
>>
>>
>> -- 
>> Regards,
>> Alexey
>>
>> [1] 
>> https://github.com/openjdk/jdk/blob/29ee7c3b70ded8cd124ca5b4a38a2aee7c39068b/test/jdk/javax/swing/regtesthelpers/Util.java#L87
>> [2] https://bugs.openjdk.org/browse/JDK-8300727
>> [3] 
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/System.html#gc()
>> [4] https://github.com/openjdk/jdk/pull/12594
>> [5] 
>> https://github.com/openjdk/jdk/blob/f612dcfebea7ffd4390f833646ad45d6f0ebd04f/test/jdk/java/lang/ref/ReferenceEnqueue.java#L54-L60
>> [6] 
>> https://github.com/openjdk/jdk/blob/f612dcfebea7ffd4390f833646ad45d6f0ebd04f/test/jdk/java/lang/ref/PhantomReferentClearing.java#L85-L92