On Wed, 19 Apr 2023 14:46:48 GMT, Afshin Zafari <d...@openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiRawMonitor.hpp line 114:
>> 
>>> 112: 
>>> 113:   // Non-aborting operator new
>>> 114:   void* operator new(size_t size, const std::nothrow_t&  
>>> nothrow_constant) throw() {
>> 
>> Hm, now I'm wondering why isn't an `operator delete` to go with this?  Or 
>> are these objects
>> never deleted?  Otherwise I'd have thought we'd get the same mismatched 
>> new/delete warning
>> you encountered elsewhere.  If they're never supposed to be deleted, then 
>> giving `operator delete`
>> a deleted definition here seems appropriate, to prevent accidentally calling 
>> the CHeapObj function.
>
> This `operator new` just calls the `CHeapObj::operator new` with nothrow 
> argument. So changing the caller will call the right one in `CHeapObj`. This 
> object is deleted in  
> https://github.com/openjdk/jdk/blob/c738c8ea3e9fda87abb03acb599a2433a344db09/src/hotspot/share/prims/jvmtiEnv.cpp#L3699
> and this will call the `CHeapObj::operator delete` which is the right one. So 
> this `operator new` is not needed since I changed the caller.

A possible reason for keeping this `operator new` is to force the use of null 
return for oom for this class.
If it's removed then we have the option of (perhaps unintentionally) using the 
terminating allocator.
That doesn't seem like a _strong_ reason to keep it, but someone more familiar 
with jvmti stuff might
want to weigh in.  If it is kept, then I think it should have a corresponding 
`operator delete`, else it at
least looks odd.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13498#discussion_r1172085220

Reply via email to