On Thu, 10 Apr 2025 22:24:04 GMT, Brent Christian <bchri...@openjdk.org> wrote:

>> Certain specific types of tests involving GC and reference processing need 
>> to account for the delay between a GC completing (during which the GC clears 
>> a Reference), and the Reference being added to its associated queue. At 
>> present, ad hoc mechanisms (with delays/timeout) are used, but can lead to 
>> intermittent test failures 
>> ([JDK-8298783](https://bugs.openjdk.org/browse/JDK-8298783)  is a recent 
>> example).
>> 
>> A better mechanism already exists in the private 
>> `Reference.waitForReferenceProcessing()` method. This PR makes 
>> `waitForReferenceProcessing()` available to tests via the `WhiteBox` and 
>> `ForceGC` test libraries.
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Separate method to setup Method, per Kim; also simplify exception handling

Marked as reviewed by kbarrett (Reviewer).

test/lib/jdk/test/whitebox/WhiteBox.java line 598:

> 596:     try {
> 597:       Method wfrp = getWaitForReferenceProcessingMethod();
> 598:       return (Boolean) wfrp.invoke(null);

I didn't know there was an implicit conversion from Boolean to boolean.  Nice!

test/lib/jdk/test/whitebox/WhiteBox.java line 603:

> 601:       throw e;
> 602:     } catch (Throwable t) {
> 603:       throw new RuntimeException(t);

The more complicated exception handling in my prototype was to make this
wrapper function behave like the wrapped function. But maybe that's
over-engineering? Although it seems wrong to me to wrap an Error in a
RuntimeException. But implementations of `invoke` are already doing the
similar thing of wrapping an Error in an InvocationTargetException (which this
code just further wraps). That also seems wrong to me, but seems to be
long-standing behavior.  Oh well.  I guess this is okay.  Certainly it's simpler
for test usage.

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

PR Review: https://git.openjdk.org/jdk/pull/24527#pullrequestreview-2759155279
PR Review Comment: https://git.openjdk.org/jdk/pull/24527#discussion_r2038806687
PR Review Comment: https://git.openjdk.org/jdk/pull/24527#discussion_r2038807296

Reply via email to