On Tue, 24 Nov 2020 03:05:19 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Please review this change to Reference.clear() to address several issues. >> >> (JDK-8240696) For GCs using a SATB barrier, simply assigning the referent >> field to null may extend the lifetime of the referent value. >> >> (JDK-8240696) For GCs with concurrent reference processing, clearing the >> referent field during reference processing may discard the expected >> notification. >> >> Both of these are addressed by introducing a private native helper function >> for clearing the referent, rather than using an ordinary in-Java field >> assignment. Tests have been added for both of these issues. This required >> adding a new breakpoint in reference processing for ZGC. >> >> Of course, finalization adds some complexity to the problem. We deal with >> that by having FinalReference override clear. The implementation is >> provided by a new package-private method in Reference. (There are a number >> of alternatives, all of them clumsy; finalization is annoying that way.) >> >> While dealing with FinalReference clearing it was noted that the recent >> JDK-8256106 and JDK-8256370 have some problems. FinalizerHistogram was not >> updated to call the new Reference.getInactive(), instead still calling get() >> on FinalReferences, with the JDK-8256106 problems. Fixing that showed the >> assertion for inactive FinalReference added by JDK-8256370 used the wrong >> test. >> >> Rather than tracking down and changing all get() and clear() calls on final >> references and changing them to use getInactive and a new similar clear >> function, I've changed FinalReference to override get and clear, which call >> the helper functions in Reference. I've also renamed getInactive to be more >> explanatory and less convenient to call directly, and similarly named the >> helper for clear. This means that get/clear should never be called on an >> active FinalReference. That's already never done, and would have problems >> if it were. >> >> Testing: >> mach5 tier1-6 >> Local (linux-x64) tier1 using Shenandoah. >> New TestReferenceClearDuringMarking fails for G1 without these changes. >> New TestReferenceClearDuringReferenceProcessing fails for ZGC without these >> changes. > > Kim Barrett has updated the pull request incrementally with two additional > commits since the last revision: > > - new tests need ref in oldgen too > - remove obsolete comment about races with clear and enqueue Looks good! ------------- Marked as reviewed by pliden (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1376
