JNI ReleasePrimitiveArrayCritical with mode JNI_COMMIT shouldn't end critical region

[Ian Rogers]

Thanks David, I think we may have differing opinions on clarity :-) In the
case of -Xcheck:jni and with HotSpot a copy is always made, and so perhaps
the attached patch causes the implementation to match the spec. Given that
commit doesn't free a copy, is it intended behavior that in HotSpot it ends
a critical region? ie In a VM that allowed copies you could imagine, Get,
Commit, Release and as spec-ed the critical region ends at the commit if
not a copy, and at the release if it is. Perhaps the simplest thing is to
remove the notion of commit here.

Thanks,
Ian

On Tue, Dec 5, 2017 at 4:37 PM, David Holmes <david.holmes at oracle.com>
wrote:

> Hi Ian,
>
> On 6/12/2017 9:15 AM, Ian Rogers wrote:
>
>> From:
>> https://docs.oracle.com/javase/8/docs/technotes/guides/jni/
>> spec/functions.html#GetPrimitiveArrayCritical_ReleasePrimiti
>> veArrayCritical
>>
>
> You need to see the updated spec:
>
> https://docs.oracle.com/javase/9/docs/specs/jni/functions.
> html#getprimitivearraycritical-releaseprimitivearraycritical
>
> That spec makes it clear:
>
> "mode: the release mode (see Primitive Array Release Modes): 0, JNI_COMMIT
> or JNI_ABORT. Ignored if carray was a not copy."
>
> In hotspot getCriticalArrayPrimitive never returns a copy so the mode is
> always ignored, as per the existing comment.
>
> David
> -----
>
>
> "The semantics of these two functions are very similar to the existing
>> Get/Release<primitivetype>ArrayElements functions. "
>>
>> https://docs.oracle.com/javase/8/docs/technotes/guides/jni/
>> spec/functions.html#Release_PrimitiveType_ArrayElements_routines
>>
>> "JNI_COMMIT copy back the content but do not free the elems buffer"
>>
>> Consider the pattern of:
>> GetPrimitiveArrayCritical(...)
>> ReleasePrimitiveArrayCritical(..., JNI_COMMIT)
>> ReleasePrimitiveArrayCritical(..., 0)
>>
>> where the first release is to just achieve a copy back. For example,
>> jniCheck.cpp will copy back but not free a copy in the case of JNI_COMMIT
>> for a critical. The implementation of ReleasePrimitiveArrayCritical
>> ignores
>> all arguments and so it ends the critical region even in the event of a
>> commit.
>>
>> The attached patch makes ReleasePrimitiveArrayCritical consider the mode
>> argument when releasing the GCLocker.
>>
>> Thanks,
>> Ian
>>
>>