RFR(xs): 8159019: ResourceMark in ClassLoader::open_versioned_entry() is being used incorrectly

[Jiangli Zhou]

Hi Ioi,

> On Jun 8, 2016, at 5:54 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> David,
> 
> There are quite a few cases where resource-allocated buffers are allocated. E.g.
> 
>  const char* Symbol::as_klass_external_name() const;
> 
> I don't think there's a rule against such usage. I think the rule should be -- if a function is returning a resource-allocated buffer, don't do the allocation within a ResourceMark declared by this function. This is exactly what's done by Calvin's fix. It looks good to me.

I had some doubts about the resource-allocated ‘buffer' in open_entry() when debugging the issue and made the suggested fix. Now David brings it up. I have to agree with him on this. In the particular case, the callers have to be very careful to not place the ResourceMark on the call path. It could be several call depths. It doesn’t seem to be a good idea to require examining all callees' code to see if resource allocated memory is returned when handling case like this.

Thanks,
Jiangli

> 
> Thanks
> - Ioi
> 
> On 6/8/16 5:19 PM, David Holmes wrote:
>> Hi Calvin,
>> 
>> On 9/06/2016 4:01 AM, Calvin Cheung wrote:
>>> 
>>> Please review this small fix for correcting the use of ResourceMark in
>>> classLoader.cpp.
>>> 
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8159019
>>> webrev: http://cr.openjdk.java.net/~ccheung/8159019/webrev.00/
>> 
>> I think this really fixes the wrong problem. I don't think ClassPathZipEntry::open_entry should be returning a resource-allocated buffer in the first place. The lifecycle management is not suitable.
>> 
>> David
>> -----
>> 
>>> Tested manually with the affected testcase on linux_x64.
>>> Running tests on all platforms.
>>> 
>>> thanks,
>>> Calvin
>