On Wed, 11 Nov 2020 11:58:08 GMT, Lance Andersen <lan...@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
