RFR (XXXS): 8193234: When using -Xcheck:jni an internally allocated buffer can leak

[Harold Seigel]

Looks good!

Harold

On 2/8/2019 1:07 AM, David Holmes wrote:
> Updated webrev:
>
> http://cr.openjdk.java.net/~dholmes/8193234/webrev.v2/
>
> Can I get a second review please.
>
> Thanks,
> David
>
> On 8/02/2019 8:59 am, David Holmes wrote:
>> Hi Aleksey,
>>
>> Thanks for looking at this.
>>
>> On 8/02/2019 1:53 am, Aleksey Shipilev wrote:
>>> On 2/7/19 11:13 AM, David Holmes wrote:
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8193234
>>>> webrev: http://cr.openjdk.java.net/~dholmes/8193234/webrev
>>>
>>> The patch looks good.
>>>
>>>> Full details in bug report but basically with the use of 
>>>> GuardedMemory in the checked versions of
>>>> the *ArrayCritical functions we always have to release the 
>>>> GuardedMemory copy.
>>>
>>> Ah, oops. It seems fine to free the copy of guarded memory 
>>> unconditionally. While you are at it,
>>> switch statement can also be simplified by coalescing case labels. 
>>> It also makes the dichotomy
>>> between "normal" and "abort" paths evident:
>>>
>>>    switch (mode) {
>>>    case 0:
>>>    case JNI_COMMIT:
>>>      memcpy(orig_result, carray, sz);
>>>      break;
>>>    case JNI_ABORT:
>>>      break;
>>>    default:
>>>      ...
>>>    }
>>
>> The coalescing is only possible because we never uses copies and so 
>> never have to free them - which is the difference between modes 0 and 
>> JNI_COMMIT. But I can do that and add a clarifying comment.
>>
>> Thanks,
>> David
>>
>>> -Aleksey
>>>