On Wed, 23 Apr 2025 00:23:57 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: > > make waitForReferenceProcessingMethod volatile A couple of lingering comments that may or may not result in changes, but otherwise looks good to me. test/lib/jdk/test/whitebox/WhiteBox.java line 598: > 596: Method wfrp = getWaitForReferenceProcessingMethod(); > 597: return (Boolean) wfrp.invoke(null); > 598: } catch (IllegalAccessException t) { s/t/e/ ? `t` seems to be leftover from when the version that was catching a Throwable. test/lib/jdk/test/whitebox/WhiteBox.java line 605: > 603: throw (InterruptedException) cause; > 604: } else { > 605: throw new RuntimeException(e); I still think it would be better to finish unwrapping e, rather than further wrapping it. We know the only checked exception is InterruptedException, because we checked that when looking up the method. That leaves either RuntimeException or Error. I think it is generally inappropriate to wrap Errors, and we don't really need to additionally wrap the RuntimeException case. But I'm going to approve this change anyway, and leave that question to real Java programmers. ------------- Marked as reviewed by kbarrett (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24527#pullrequestreview-2786037305 PR Review Comment: https://git.openjdk.org/jdk/pull/24527#discussion_r2055313446 PR Review Comment: https://git.openjdk.org/jdk/pull/24527#discussion_r2055314679
