RFR (M): 8238854: Remove superfluous C heap allocation failure checks

[Thomas Schatzl]

Hi Stefan,

   thanks for your review.

On 12.02.20 12:38, Stefan Johansson wrote:
> Hi Thomas,
> 
>> 11 feb. 2020 kl. 14:30 skrev Thomas Schatzl <thomas.schatzl at oracle.com>:
>>
>> Hi all,
>>
>>   can I have reviews for this change that removes superfluous C heap allocation failure checks (basically hard-exiting the VM) because by default C allocation already exits the VM.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8238854
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8238854/webrev/
> Nice cleanup in general, but MemRegion doesn’t derive from CHeapObj and will return NULL on failure:
> void* MemRegion::operator new(size_t size) throw() {
>    return (address)AllocateHeap(size, mtGC, CURRENT_PC,
>      AllocFailStrategy::RETURN_NULL);
> }
> 
> So we should either change this to use the default AllocFailStrategy or keep the checks.
> 

Nice catch. I opted to revert the changes for MemRegion allocation.

Although I think all users of new for MemRegion expect it to fail (the 
only other user in filemap.cpp will crash with NPE a few lines after 
allocation), this needs more investigation because the change 
introducing the new operator talks about some clang compatibility issue 
(from 2003). But it also indicates that the problem occurred only on a 
use that is not in the code base any more (JDK-8021954 ftr, it is a 
closed issue that can't be opened).

(Note that my testing did not reproduce the failure, but, the code is 
not used in the crashing component, i.e. metaspace handling, any more).

http://cr.openjdk.java.net/~tschatzl/8238854/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8238854/webrev.1/ (full)

Thanks,
   Thomas