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

[David Holmes]

Thanks Harold!

David

On 9/02/2019 7:33 am, Harold Seigel wrote:
> 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
>>>>