On Thu, 20 Apr 2023 05:05:17 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:

>> 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.

JVMTI does not abort on OOM it reports an error, so we definitely do not want a 
terminating allocator!

jvmtiError
JvmtiEnv::CreateRawMonitor(const char* name, jrawMonitorID* monitor_ptr) {
  JvmtiRawMonitor* rmonitor = new JvmtiRawMonitor(name);
  NULL_CHECK(rmonitor, JVMTI_ERROR_OUT_OF_MEMORY);

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

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

Reply via email to