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

Ioi Lam ioi.lam at oracle.com
Tue Feb 18 18:00:28 UTC 2020


The changes look OK to me.

I think this line doesn't need to be changed.

1820   *heap_mem = cleanup._regions;

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




More information about the hotspot-gc-dev mailing list