RFR (S): 8239070: Memory leak when unsuccessfully mapping in archive regions

[Kim Barrett]

> On Feb 18, 2020, at 1:00 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> The changes look OK to me.
> 
> I think this line doesn't need to be changed.
> 
> 1820   *heap_mem = cleanup._regions;

+1

I don’t need a new webrev for revert of line 1820.

> 
> Thanks
> - Ioi
> 
> On 2/18/20 8:03 AM, Thomas Schatzl wrote:
>> Hi,
>> 
>> On 15.02.20 09:20, Kim Barrett wrote:
>>>> On Feb 14, 2020, at 11:08 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>>> 
>>>> Hi Thomas,
>>>> 
>>>> Thanks for fixing this issue. Freeing the array at each exit point seems error prone. How about: refactoring the function to a FileMapInfo::map_heap_data_impl function, allocate inside FileMapInfo::map_heap_data(), call map_heap_data() and if it returns false, free the array in a single place.
>>> 
>>> Rather than splitting up the function, one could add a local cleanup handler:
>>> 
>>>    ... create and initialize regions object ...
>>>    struct Cleanup {
>>>      MemRegion* _regions;
>>>      bool _aborted;
>>>      Cleanup(MemRegion* regions) : _regions(regions), _aborted(true) {}
>>>      ~Cleanup() { if (_aborted) FREE_C_HEAP_ARRAY(MemRegion, _regions); }
>>>    } cleanup(regions);
>>>    ...
>>>    cleanup._aborted = false;
>>>    return true;
>>> }
>>> 
>> 
>>   I implemented that as it is least intrusive in the end.
>> 
>> http://cr.openjdk.java.net/~tschatzl/8239070/webrev.1/ (full, no point in providing diff)
>> 
>> Thanks,
>>   Thomas