RFR: 8256018: Adler32/CRC32/CRC32C missing reachabilityFence [v2]

[Volker Simonis]

On Wed, 11 Nov 2020 11:58:08 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> Hi, 
>> 
>> Please review the fix for JDK-8256018 which addresses the issue that  the update(ByteBuffer)  methods of Adler32, CRC32, and CRC32C should use Reference.reachabilityFence to ensure that direct byte buffer are kept kept alive when they are accessed directly.
>> 
>> The Mach5 jdk-tier1, jdk-tier2, jdk-tier3 as well as the  jck:api/java_util/zip,jck:api/java_util/jar continue to run clean.
>> 
>> Best
>> Lance
>
> Lance Andersen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove extra blank line

I wonder why this change was necessary? Did you see any concrete problems or crashes? If yes, could you please share any details?

The code you've fixed with a `reachabilityFence` looks as follows:

        if (buffer.isDirect()) {
            try {
                crc = updateByteBuffer(crc, ((DirectBuffer)buffer).address(), pos, rem);
            } finally {
                Reference.reachabilityFence(buffer);
            }
        } else if (buffer.hasArray()) {
            ...
        } else {
            ...
        }
        buffer.position(limit); // <-- 'buffer' is still alive here (at least in the bytecode)

But as you can see, `buffer` is still alive at the end of the method. According to @iwanowww (see [this mail](http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051312.html) during the ["Reduce Chance Of Mistakenly Early Backing Memory Cleanup" discussion](http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/thread.html#51221)) this should suffice because HotSpot computes reachability based on bytecode analysis (rather than analysis of optimized IR). 

Shouldn't this be enough to keep `buffer` alive until `updateByteBuffer()` has finished working on the buffer?

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

PR: https://git.openjdk.java.net/jdk/pull/1149